* Re: [PATCH v3] pseries/drmem: update LMBs after LPM
From: Tyrel Datwyler @ 2021-04-30 23:58 UTC (permalink / raw)
To: Laurent Dufour, Aneesh Kumar K.V, mpe, benh, paulus
Cc: nathanl, linuxppc-dev, linux-kernel
In-Reply-To: <271ef351-b89c-ba68-3b6d-baa24cc0021b@linux.ibm.com>
On 4/30/21 9:13 AM, Laurent Dufour wrote:
> Le 29/04/2021 à 21:12, Tyrel Datwyler a écrit :
>> On 4/29/21 3:27 AM, Aneesh Kumar K.V wrote:
>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>
>>>> After a LPM, the device tree node ibm,dynamic-reconfiguration-memory may be
>>>> updated by the hypervisor in the case the NUMA topology of the LPAR's
>>>> memory is updated.
>>>>
>>>> This is caught by the kernel, but the memory's node is updated because
>>>> there is no way to move a memory block between nodes.
>>>>
>>>> If later a memory block is added or removed, drmem_update_dt() is called
>>>> and it is overwriting the DT node to match the added or removed LMB. But
>>>> the LMB's associativity node has not been updated after the DT node update
>>>> and thus the node is overwritten by the Linux's topology instead of the
>>>> hypervisor one.
>>>>
>>>> Introduce a hook called when the ibm,dynamic-reconfiguration-memory node is
>>>> updated to force an update of the LMB's associativity.
>>>>
>>>> Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>> ---
>>>>
>>>> V3:
>>>> - Check rd->dn->name instead of rd->dn->full_name
>>>> V2:
>>>> - Take Tyrel's idea to rely on OF_RECONFIG_UPDATE_PROPERTY instead of
>>>> introducing a new hook mechanism.
>>>> ---
>>>> arch/powerpc/include/asm/drmem.h | 1 +
>>>> arch/powerpc/mm/drmem.c | 35 +++++++++++++++++++
>>>> .../platforms/pseries/hotplug-memory.c | 4 +++
>>>> 3 files changed, 40 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/drmem.h
>>>> b/arch/powerpc/include/asm/drmem.h
>>>> index bf2402fed3e0..4265d5e95c2c 100644
>>>> --- a/arch/powerpc/include/asm/drmem.h
>>>> +++ b/arch/powerpc/include/asm/drmem.h
>>>> @@ -111,6 +111,7 @@ int drmem_update_dt(void);
>>>> int __init
>>>> walk_drmem_lmbs_early(unsigned long node, void *data,
>>>> int (*func)(struct drmem_lmb *, const __be32 **, void *));
>>>> +void drmem_update_lmbs(struct property *prop);
>>>> #endif
>>>> static inline void invalidate_lmb_associativity_index(struct drmem_lmb
>>>> *lmb)
>>>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>>>> index 9af3832c9d8d..f0a6633132af 100644
>>>> --- a/arch/powerpc/mm/drmem.c
>>>> +++ b/arch/powerpc/mm/drmem.c
>>>> @@ -307,6 +307,41 @@ int __init walk_drmem_lmbs_early(unsigned long node,
>>>> void *data,
>>>> return ret;
>>>> }
>>>> +/*
>>>> + * Update the LMB associativity index.
>>>> + */
>>>> +static int update_lmb(struct drmem_lmb *updated_lmb,
>>>> + __maybe_unused const __be32 **usm,
>>>> + __maybe_unused void *data)
>>>> +{
>>>> + struct drmem_lmb *lmb;
>>>> +
>>>> + /*
>>>> + * Brut force there may be better way to fetch the LMB
>>>> + */
>>>> + for_each_drmem_lmb(lmb) {
>>>> + if (lmb->drc_index != updated_lmb->drc_index)
>>>> + continue;
>>>> +
>>>> + lmb->aa_index = updated_lmb->aa_index;
>>>> + break;
>>>> + }
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Update the LMB associativity index.
>>>> + *
>>>> + * This needs to be called when the hypervisor is updating the
>>>> + * dynamic-reconfiguration-memory node property.
>>>> + */
>>>> +void drmem_update_lmbs(struct property *prop)
>>>> +{
>>>> + if (!strcmp(prop->name, "ibm,dynamic-memory"))
>>>> + __walk_drmem_v1_lmbs(prop->value, NULL, NULL, update_lmb);
>>>> + else if (!strcmp(prop->name, "ibm,dynamic-memory-v2"))
>>>> + __walk_drmem_v2_lmbs(prop->value, NULL, NULL, update_lmb);
>>>> +}
>>>> #endif
>>>> static int init_drmem_lmb_size(struct device_node *dn)
>>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>> b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>> index 8377f1f7c78e..672ffbee2e78 100644
>>>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>>> @@ -949,6 +949,10 @@ static int pseries_memory_notifier(struct
>>>> notifier_block *nb,
>>>> case OF_RECONFIG_DETACH_NODE:
>>>> err = pseries_remove_mem_node(rd->dn);
>>>> break;
>>>> + case OF_RECONFIG_UPDATE_PROPERTY:
>>>> + if (!strcmp(rd->dn->name,
>>>> + "ibm,dynamic-reconfiguration-memory"))
>>>> + drmem_update_lmbs(rd->prop);
>>>> }
>>>> return notifier_from_errno(err);
>>>
>>> How will this interact with DLPAR memory? When we dlpar memory,
>>> ibm,configure-connector is used to fetch the new associativity details
>>> and set drmem_lmb->aa_index correctly there. Once that is done kernel
>>> then call drmem_update_dt() which will result in the above notifier
>>> callback?
>>>
>>> IIUC, the call back then will update drmem_lmb->aa_index again?
>>
>> After digging through some of this code I'm a bit concerned about all the kernel
>> device tree manipulation around memory DLPAR both with the assoc-lookup-array
>> prop update and post dynamic-memory prop updating. We build a drmem_info array
>> of the LMBs from the device-tree at boot. I don't really understand why we are
>> manipulating the device tree property every time we add/remove an LMB. Not sure
>> the reasoning was to write back in particular the aa_index and flags for each
>> LMB into the device tree when we already have them in the drmem_info array. On
>> the other hand the assoc-lookup-array I suppose would need to have an in kernel
>> representation to avoid updating the device tree property every time.
>
> I think the reason is to keep the device tree in sync with the current set of LMBs.
I don't really think that is how the device tree is meant to be used. We have an
in memory representation of the LMBs separate from the device tree, and that is
were we should track OS specific state. The values in the device-tree property
can be updated via device node remove/add or update-properties RTAS call. These
are the means that the platform reports (OS discovers) underlying changes. The
new property is going to blow away any previous state that the OS wrote there.
This is likely, one of the culprits of memory DLPAR problems that have been
observed after LPM.
>
> My understanding is that the kernel is not really using the
> 'ibm,dynamic-memory*' DT property once the boot is done. But user space tools
> (like lsslot and drmgr) read it to built the LMB tree and get the DRC index for
> each LMBs as it is not available in SYSFS.
Yeah, but as I mentioned above the property can change as a result of an update
we process from the hypervisor in response to something like LPM (or PRRN if we
ever figure out how to make that work correctly). So, if there is some sort of
state drmgr needs to know we have to figure out a different way to expose that
information.
>
>> Changes to the device tree should be things reported to the system from the
>> hypervisor through the proper interfaces, and as a result any code that cares
>> can register an of_reconfig_notifier to resepond to device tree updates. The
>> memory dlpar code seems to be needlessly manipulating the device-tree which
>> leads to the problem here where a notifier callback is now duplicating work.
>
> I don't think the hypervisor is expected to update the 'ibm,dynamic-memory' each
> time a LMB is added, this is not design this way AFAIK.
It shouldn't need to for the most part. The only information that should change
in ibm,dynamic-memory in the first place is the aa_index as a result of an
underlying platform reassignment. The flags in ibm,dynamic-memory aren't
changing as a result of DLPAR add/remove. The aa_index could be out of date as I
mentioned above. The use of DRCONF_MEM_ASSIGNED in my opinion is actually a hack
to mark LMBs as present and owned by the partition. Its actual PAPR definition
is soley to identify LMBs that are present at boot.
As of today I don't have a problem with your patch. This was more of me pointing
out things that I think are currently wrong with our memory hotplug
implementation, and that we need to take a long hard look at it down the road.
-Tyrel
>
> Laurent.
>
>> Just my two cents FWIW.
>>
>> -Tyrel
>>
>>>
>>> -aneesh
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/pseries: Fix hcall tracing recursion in pv queued spinlocks
From: Nicholas Piggin @ 2021-05-01 1:22 UTC (permalink / raw)
To: linuxppc-dev, Naveen N. Rao
In-Reply-To: <1619529780.yjjzv9cw5m.naveen@linux.ibm.com>
Excerpts from Naveen N. Rao's message of April 27, 2021 11:43 pm:
> Nicholas Piggin wrote:
>> The paravit queued spinlock slow path adds itself to the queue then
>> calls pv_wait to wait for the lock to become free. This is implemented
>> by calling H_CONFER to donate cycles.
>>
>> When hcall tracing is enabled, this H_CONFER call can lead to a spin
>> lock being taken in the tracing code, which will result in the lock to
>> be taken again, which will also go to the slow path because it queues
>> behind itself and so won't ever make progress.
>>
>> An example trace of a deadlock:
>>
>> __pv_queued_spin_lock_slowpath
>> trace_clock_global
>> ring_buffer_lock_reserve
>> trace_event_buffer_lock_reserve
>> trace_event_buffer_reserve
>> trace_event_raw_event_hcall_exit
>> __trace_hcall_exit
>> plpar_hcall_norets_trace
>> __pv_queued_spin_lock_slowpath
>> trace_clock_global
>> ring_buffer_lock_reserve
>> trace_event_buffer_lock_reserve
>> trace_event_buffer_reserve
>> trace_event_raw_event_rcu_dyntick
>> rcu_irq_exit
>> irq_exit
>> __do_irq
>> call_do_irq
>> do_IRQ
>> hardware_interrupt_common_virt
>>
>> Fix this by introducing plpar_hcall_norets_notrace(), and using that to
>> make SPLPAR virtual processor dispatching hcalls by the paravirt
>> spinlock code.
>>
>> Fixes: 20c0e8269e9d ("powerpc/pseries: Implement paravirt qspinlocks for SPLPAR")
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/hvcall.h | 3 +++
>> arch/powerpc/include/asm/paravirt.h | 22 +++++++++++++++++++---
>> arch/powerpc/platforms/pseries/hvCall.S | 10 ++++++++++
>> arch/powerpc/platforms/pseries/lpar.c | 4 ++--
>> 4 files changed, 34 insertions(+), 5 deletions(-)
>
> Thanks for the fix! Some very minor nits below, but none the less:
> Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>
>>
>> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
>> index ed6086d57b22..0c92b01a3c3c 100644
>> --- a/arch/powerpc/include/asm/hvcall.h
>> +++ b/arch/powerpc/include/asm/hvcall.h
>> @@ -446,6 +446,9 @@
>> */
>> long plpar_hcall_norets(unsigned long opcode, ...);
>>
>> +/* Variant which does not do hcall tracing */
>> +long plpar_hcall_norets_notrace(unsigned long opcode, ...);
>> +
>> /**
>> * plpar_hcall: - Make a pseries hypervisor call
>> * @opcode: The hypervisor call to make.
>> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
>> index 5d1726bb28e7..3c13c2ec70a9 100644
>> --- a/arch/powerpc/include/asm/paravirt.h
>> +++ b/arch/powerpc/include/asm/paravirt.h
>> @@ -30,17 +30,33 @@ static inline u32 yield_count_of(int cpu)
>>
>> static inline void yield_to_preempted(int cpu, u32 yield_count)
>> {
>
> It looks like yield_to_preempted() is only used by simple spin locks
> today. I wonder if it makes more sense to put the below comment in
> yield_to_any() which is used by the qspinlock code.
Yeah, I just put it above the functions entirely because it refers to
all of them.
>
>> - plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_count);
>> + /*
>> + * Spinlock code yields and prods, so don't trace the hcalls because
>> + * tracing code takes spinlocks which could recurse.
>> + *
>> + * These calls are made while the lock is not held, the lock slowpath
>> + * yields if it can not acquire the lock, and unlock slow path might
>> + * prod if a waiter has yielded). So this did not seem to be a problem
>> + * for simple spin locks because technically it didn't recuse on the
> ^^^^^^
> recurse
>
>> + * lock. However the queued spin lock contended path is more strictly
>> + * ordered: the H_CONFER hcall is made after the task has queued itself
>> + * on the lock, so then recursing on the lock will queue up behind that
>> + * (or worse: queued spinlocks uses tricks that assume a context never
>> + * waits on more than one spinlock, so that may cause random
>> + * corruption).
>> + */
>> + plpar_hcall_norets_notrace(H_CONFER,
>> + get_hard_smp_processor_id(cpu), yield_count);
>
> This can all be on a single line.
Should it though? Linux in general allegedly changed to 100 column
lines for checkpatch, but it seems to still be frowned upon to go
beyond 80 deliberately. What about arch/powerpc?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries: warn if recursing into the hcall tracing code
From: Nicholas Piggin @ 2021-05-01 1:24 UTC (permalink / raw)
To: linuxppc-dev, Naveen N. Rao
In-Reply-To: <1619531703.lv0qigovgz.naveen@linux.ibm.com>
Excerpts from Naveen N. Rao's message of April 27, 2021 11:59 pm:
> Nicholas Piggin wrote:
>> ---
>> arch/powerpc/platforms/pseries/lpar.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
>> index 835e7f661a05..a961a7ebeab3 100644
>> --- a/arch/powerpc/platforms/pseries/lpar.c
>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>> @@ -1828,8 +1828,11 @@ void hcall_tracepoint_unregfunc(void)
>>
>> /*
>> * Since the tracing code might execute hcalls we need to guard against
>> - * recursion. H_CONFER from spin locks must be treated separately though
>> - * and use _notrace plpar_hcall variants, see yield_to_preempted().
>> + * recursion, but this always seems risky -- __trace_hcall_entry might be
>> + * ftraced, for example. So warn in this case.
>
> __trace_hcall_[entry|exit] aren't traced anymore since they now have the
> 'notrace' annotation.
Yes that's true I went back and added the other patch, so I should fix
this comment.
>> + *
>> + * H_CONFER from spin locks must be treated separately though and use _notrace
>> + * plpar_hcall variants, see yield_to_preempted().
>> */
>> static DEFINE_PER_CPU(unsigned int, hcall_trace_depth);
>>
>> @@ -1843,7 +1846,7 @@ notrace void __trace_hcall_entry(unsigned long opcode, unsigned long *args)
>>
>> depth = this_cpu_ptr(&hcall_trace_depth);
>>
>> - if (*depth)
>> + if (WARN_ON_ONCE(*depth))
>> goto out;
>
> I don't think this will be helpful. The hcall trace depth tracking is
> for the tracepoint and I suspect that this warning will be triggered
> quite easily. Since we have recursion protection, I don't think we
> should warn here.
What would trigger recursion?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Nicholas Piggin @ 2021-05-01 1:58 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210415230948.3563415-2-farosas@linux.ibm.com>
Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
> As one of the arguments of the H_ENTER_NESTED hypercall, the nested
> hypervisor (L1) prepares a structure containing the values of various
> hypervisor-privileged registers with which it wants the nested guest
> (L2) to run. Since the nested HV runs in supervisor mode it needs the
> host to write to these registers.
>
> To stop a nested HV manipulating this mechanism and using a nested
> guest as a proxy to access a facility that has been made unavailable
> to it, we have a routine that sanitises the values of the HV registers
> before copying them into the nested guest's vcpu struct.
>
> However, when coming out of the guest the values are copied as they
> were back into L1 memory, which means that any sanitisation we did
> during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
>
> This patch alters this sanitisation to have effect on the vcpu->arch
> registers directly before entering and after exiting the guest,
> leaving the structure that is copied back into L1 unchanged (except
> when we really want L1 to access the value, e.g the Cause bits of
> HFSCR).
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
> 1 file changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 0cd0e7aad588..270552dd42c5 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> {
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>
> + /*
> + * When loading the hypervisor-privileged registers to run L2,
> + * we might have used bits from L1 state to restrict what the
> + * L2 state is allowed to be. Since L1 is not allowed to read
> + * the HV registers, do not include these modifications in the
> + * return state.
> + */
> + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
> + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
> +
> hr->dpdes = vc->dpdes;
> - hr->hfscr = vcpu->arch.hfscr;
> hr->purr = vcpu->arch.purr;
> hr->spurr = vcpu->arch.spurr;
> hr->ic = vcpu->arch.ic;
Do we still have the problem here that hfac interrupts due to bits cleared
by the hfscr sanitisation would have the cause bits returned to the L1,
so in theory it could probe hfscr directly that way? I don't see a good
solution to this except either have the L0 intercept these faults and do
"something" transparent, or return error from H_ENTER_NESTED (which would
also allow trivial probing of the facilities).
Returning an hfac interrupt to a hypervisor that thought it enabled the
bit would be strange. But so does appearing to modify the register
underneath it and then returning a fault.
I think the sanest thing would actually be to return failure from the
hcall.
> @@ -132,24 +141,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> }
> }
>
> -static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> -{
> - /*
> - * Don't let L1 enable features for L2 which we've disabled for L1,
> - * but preserve the interrupt cause field.
> - */
> - hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
> -
> - /* Don't let data address watchpoint match in hypervisor state */
> - hr->dawrx0 &= ~DAWRX_HYP;
> - hr->dawrx1 &= ~DAWRX_HYP;
> -
> - /* Don't let completed instruction address breakpt match in HV state */
> - if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> - hr->ciabr &= ~CIABR_PRIV;
> -}
> -
> -static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> +static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state *hr)
> {
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>
> @@ -261,6 +253,27 @@ static int kvmhv_write_guest_state_and_regs(struct kvm_vcpu *vcpu,
> sizeof(struct pt_regs));
> }
>
> +static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
> + const struct hv_guest_state *l2_hv,
> + const struct hv_guest_state *l1_hv)
> +{
> + restore_hv_regs(vcpu, l2_hv);
> +
> + /*
> + * Don't let L1 enable features for L2 which we've disabled for L1,
> + * but preserve the interrupt cause field.
> + */
> + vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
> +
> + /* Don't let data address watchpoint match in hypervisor state */
> + vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
> + vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
> +
> + /* Don't let completed instruction address breakpt match in HV state */
> + if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> + vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
> +}
> +
> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> {
> long int err, r;
> @@ -324,8 +337,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
> LPCR_LPES | LPCR_MER;
> lpcr = (vc->lpcr & ~mask) | (l2_hv.lpcr & mask);
> - sanitise_hv_regs(vcpu, &l2_hv);
> - restore_hv_regs(vcpu, &l2_hv);
> +
> + load_l2_hv_regs(vcpu, &l2_hv, &saved_l1_hv);
>
> vcpu->arch.ret = RESUME_GUEST;
> vcpu->arch.trap = 0;
> --
> 2.29.2
>
>
^ permalink raw reply
* Re: [PATCH v3 2/2] KVM: PPC: Book3S HV: Stop forwarding all HFSCR cause bits to L1
From: Nicholas Piggin @ 2021-05-01 2:04 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210415230948.3563415-3-farosas@linux.ibm.com>
Oh sorry, I didn't skim this one before replying to the first.
Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
> Since commit 73937deb4b2d ("KVM: PPC: Book3S HV: Sanitise hv_regs on
> nested guest entry") we have been disabling for the nested guest the
> hypervisor facility bits that its nested hypervisor don't have access
> to.
>
> If the nested guest tries to use one of those facilities, the hardware
> will cause a Hypervisor Facility Unavailable interrupt. The HFSCR
> register is modified by the hardware to contain information about the
> cause of the interrupt.
>
> We have been returning the cause bits to the nested hypervisor but
> since commit 549e29b458c5 ("KVM: PPC: Book3S HV: Sanitise vcpu
> registers in nested path") we are reducing the amount of information
> exposed to L1, so it seems like a good idea to restrict some of the
> cause bits as well.
>
> With this patch the L1 guest will be allowed to handle only the
> interrupts caused by facilities it has disabled for L2. The interrupts
> caused by facilities that L0 denied will cause a Program Interrupt in
> L1.
I'm not sure if this is a good solution. This would be randomly killing
guest processes or kernels with no way for them to understand what's going
on or deal with it.
The problem is really a nested hypervisor mismatch / configuration
error, so it should be handled between the L0 and L1. Returning failure
from H_ENTER_NESTED, for example (which is probe-able, but not really
any less probe-able than this approach).
Thanks,
Nick
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 270552dd42c5..912a2bcdf7b0 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -138,6 +138,23 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> hr->heir = vcpu->arch.emul_inst;
> break;
> + case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
> + {
> + u8 cause = vcpu->arch.hfscr >> 56;
> +
> + WARN_ON_ONCE(cause >= BITS_PER_LONG);
> +
> + if (hr->hfscr & (1UL << cause)) {
> + hr->hfscr &= ~HFSCR_INTR_CAUSE;
> + /*
> + * We have not restored L1 state yet, so queue
> + * this interrupt instead of delivering it
> + * immediately.
> + */
> + kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_PROGRAM);
> + }
> + break;
> + }
> }
> }
>
> --
> 2.29.2
>
>
^ permalink raw reply
* Re: [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Nicholas Piggin @ 2021-05-01 4:48 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210415230948.3563415-2-farosas@linux.ibm.com>
Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
> As one of the arguments of the H_ENTER_NESTED hypercall, the nested
> hypervisor (L1) prepares a structure containing the values of various
> hypervisor-privileged registers with which it wants the nested guest
> (L2) to run. Since the nested HV runs in supervisor mode it needs the
> host to write to these registers.
>
> To stop a nested HV manipulating this mechanism and using a nested
> guest as a proxy to access a facility that has been made unavailable
> to it, we have a routine that sanitises the values of the HV registers
> before copying them into the nested guest's vcpu struct.
>
> However, when coming out of the guest the values are copied as they
> were back into L1 memory, which means that any sanitisation we did
> during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
>
> This patch alters this sanitisation to have effect on the vcpu->arch
> registers directly before entering and after exiting the guest,
> leaving the structure that is copied back into L1 unchanged (except
> when we really want L1 to access the value, e.g the Cause bits of
> HFSCR).
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
> 1 file changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 0cd0e7aad588..270552dd42c5 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> {
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>
> + /*
> + * When loading the hypervisor-privileged registers to run L2,
> + * we might have used bits from L1 state to restrict what the
> + * L2 state is allowed to be. Since L1 is not allowed to read
> + * the HV registers, do not include these modifications in the
> + * return state.
> + */
> + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
> + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
> +
> hr->dpdes = vc->dpdes;
> - hr->hfscr = vcpu->arch.hfscr;
> hr->purr = vcpu->arch.purr;
> hr->spurr = vcpu->arch.spurr;
> hr->ic = vcpu->arch.ic;
The below parts of the patch I have no problem with, I think it's good to
be able to restore the hv_guest_state for return, e.g., for cases where
the L0 might emulate some HV behaviour transparently it will be useful,
at least.
Thanks,
Nick
> @@ -132,24 +141,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> }
> }
>
> -static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> -{
> - /*
> - * Don't let L1 enable features for L2 which we've disabled for L1,
> - * but preserve the interrupt cause field.
> - */
> - hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
> -
> - /* Don't let data address watchpoint match in hypervisor state */
> - hr->dawrx0 &= ~DAWRX_HYP;
> - hr->dawrx1 &= ~DAWRX_HYP;
> -
> - /* Don't let completed instruction address breakpt match in HV state */
> - if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> - hr->ciabr &= ~CIABR_PRIV;
> -}
> -
> -static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> +static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state *hr)
> {
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>
> @@ -261,6 +253,27 @@ static int kvmhv_write_guest_state_and_regs(struct kvm_vcpu *vcpu,
> sizeof(struct pt_regs));
> }
>
> +static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
> + const struct hv_guest_state *l2_hv,
> + const struct hv_guest_state *l1_hv)
> +{
> + restore_hv_regs(vcpu, l2_hv);
> +
> + /*
> + * Don't let L1 enable features for L2 which we've disabled for L1,
> + * but preserve the interrupt cause field.
> + */
> + vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
> +
> + /* Don't let data address watchpoint match in hypervisor state */
> + vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
> + vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
> +
> + /* Don't let completed instruction address breakpt match in HV state */
> + if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> + vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
> +}
> +
> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> {
> long int err, r;
> @@ -324,8 +337,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
> LPCR_LPES | LPCR_MER;
> lpcr = (vc->lpcr & ~mask) | (l2_hv.lpcr & mask);
> - sanitise_hv_regs(vcpu, &l2_hv);
> - restore_hv_regs(vcpu, &l2_hv);
> +
> + load_l2_hv_regs(vcpu, &l2_hv, &saved_l1_hv);
>
> vcpu->arch.ret = RESUME_GUEST;
> vcpu->arch.trap = 0;
> --
> 2.29.2
>
>
^ permalink raw reply
* RE: Radeon NI: GIT kernel with the nislands_smc commit doesn't boot on a Freescale P5040 board and P.A.Semi Nemo board
From: Deucher, Alexander @ 2021-04-30 15:26 UTC (permalink / raw)
To: Christian Zigotzky, gustavoars@kernel.org,
amd-gfx@lists.freedesktop.org
Cc: Darren Stevens, R.T.Dickinson, Gustavo A. R. Silva,
Michel Dänzer, Maling list - DRI developers, mad skateman,
Olof Johansson, linuxppc-dev, Christian Zigotzky
In-Reply-To: <3eedbe78-1fbd-4763-a7f3-ac5665e76a4a@xenosoft.de>
[AMD Public Use]
+ Gustavo, amd-gfx
> -----Original Message-----
> From: Christian Zigotzky <chzigotzky@xenosoft.de>
> Sent: Friday, April 30, 2021 8:00 AM
> To: gustavoars@kernel.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>
> Cc: R.T.Dickinson <rtd2@xtra.co.nz>; Darren Stevens <darren@stevens-
> zone.net>; mad skateman <madskateman@gmail.com>; linuxppc-dev
> <linuxppc-dev@lists.ozlabs.org>; Olof Johansson <olof@lixom.net>;
> Maling list - DRI developers <dri-devel@lists.freedesktop.org>; Michel
> Dänzer <michel@daenzer.net>; Christian Zigotzky <info@xenosoft.de>
> Subject: Radeon NI: GIT kernel with the nislands_smc commit doesn't
> boot on a Freescale P5040 board and P.A.Semi Nemo board
>
> Hello,
>
> The Nemo board (A-EON AmigaOne X1000) [1] and the FSL P5040 Cyrus+
> board (A-EON AmigaOne X5000) [2] with installed AMD Radeon HD6970 NI
> graphics cards (Cayman XT) [3] don't boot with the latest git kernel
> anymore after the commit "drm/radeon/nislands_smc.h: Replace
> one-element array with flexible-array member in struct NISLANDS_SMC_SWSTATE branch" [4].
> This git kernel boots in a virtual e5500 QEMU machine with a VirtIO-GPU [5].
>
> I bisected today [6].
>
> Result: drm/radeon/nislands_smc.h: Replace one-element array with
> flexible-array member in struct NISLANDS_SMC_SWSTATE branch
> (434fb1e7444a2efc3a4ebd950c7f771ebfcffa31) [4] is the first bad commit.
>
> I was able to revert this commit [7] and after a new compiling, the
> kernel boots without any problems on my AmigaOnes.
>
> After that I created a patch for reverting this commit for new git test kernels.
> [3]
>
> The kernel compiles and boots with this patch on my AmigaOnes. Please
> find attached the kernel config files.
>
> Please check the first bad commit.
>
> Thanks,
> Christian
>
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.
> wikipedia.org%2Fwiki%2FAmigaOne_X1000&data=04%7C01%7Calexand
> er.deucher%40amd.com%7C0622549383fb4320346b08d90bcf7be1%7C3dd89
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637553808670161651%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> 1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PNSrApUdMrku20hH7dEKlJJ
> TBi7Qp5JOkqpA4MvKqdE%3D&reserved=0
> [2]
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwiki.
> a miga.org%2Findex.php%3Ftitle%3DX5000&data=04%7C01%7Calexander
> .deucher%40amd.com%7C0622549383fb4320346b08d90bcf7be1%7C3dd8961f
> e4884e608e11a82d994e183d%7C0%7C0%7C637553808670161651%7CUnknow
> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLCJXVCI6Mn0%3D%7C1000&sdata=B8Uvhs25%2FP3RfnL1AgICN3Y4
> CEXeCE1yIoi3vvwvGto%3D&reserved=0
> [3]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fforu
> m.hyperion-
> entertainment.com%2Fviewtopic.php%3Ff%3D35%26t%3D4377&data=
> 04%7C01%7Calexander.deucher%40amd.com%7C0622549383fb4320346b08d
> 90bcf7be1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63755380
> 8670161651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TokXplD
> Tvg3%2BZMPLCgR1fs%2BN2X9MIfLXLW67MAM2Qsk%3D&reserved=0
> [4]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> k ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%
> 2Fcommit%2F%3Fid%3D434fb1e7444a2efc3a4ebd950c7f771ebfcffa31&d
> ata=04%7C01%7Calexander.deucher%40amd.com%7C0622549383fb4320346
> b08d90bcf7be1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6375
> 53808670161651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=JC
> M4xvPEnWdckcTPbQ2Ujv%2FAiMMsFMzzl4Pr%2FRPlcMQ%3D&reserve
> d=0
> [5] qemu-system-ppc64 -M ppce500 -cpu e5500 -m 1024 -kernel uImage -
> drive format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev
> user,id=mynet0 -device virtio-net-pci,netdev=mynet0 -append "rw
> root=/dev/vda" -device virtio-vga -usb -device usb-ehci,id=ehci
> -device usb- tablet -device virtio-keyboard-pci -smp 4 -vnc :1 [6]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fforu
> m.hyperion-
> entertainment.com%2Fviewtopic.php%3Fp%3D53074%23p53074&data
> =04%7C01%7Calexander.deucher%40amd.com%7C0622549383fb4320346b08
> d90bcf7be1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6375538
> 08670161651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RXfSlY
> A3bDEFas0%2Fk2vMWsl2l0nuhS2ecjZgSBLc%2Bs4%3D&reserved=0
> [7] git revert 434fb1e7444a2efc3a4ebd950c7f771ebfcffa3
^ permalink raw reply
* Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions
From: Christophe Leroy @ 2021-05-01 7:31 UTC (permalink / raw)
To: Daniel Walker, linuxppc-dev, x86, Andrew Morton
Cc: linux-kernel, xe-linux-external
In-Reply-To: <1929b3a8-f882-c930-4b99-10c6a8f127c7@csgroup.eu>
Le 30/04/2021 à 10:50, Christophe Leroy a écrit :
>
>
> Le 30/04/2021 à 10:47, Christophe Leroy a écrit :
>>
>>
>> Le 30/04/2021 à 06:22, Daniel Walker a écrit :
>>> This systems allows some string functions to be moved into
>>> lib/early_string.c and they will be prepended with "early_" and compiled
>>> without debugging like KASAN.
>>>
>>> This is already done on x86 for,
>>> "AMD Secure Memory Encryption (SME) support"
>>>
>>> and on powerpc prom_init.c , and EFI's libstub.
>>>
>>> The AMD memory feature disabled KASAN for all string functions, and
>>> prom_init.c and efi libstub implement their own versions of the
>>> functions.
>>>
>>> This implementation allows sharing of the string functions without
>>> removing the debugging features for the whole system.
>>
>> This looks good. I prefer that rather than the way you proposed to do it two years ago.
>>
>> Only one problem, see below.
>>
>>> +size_t strlcat(char *dest, const char *src, size_t count)
>>> +{
>>> + size_t dsize = strlen(dest);
>>> + size_t len = strlen(src);
>>> + size_t res = dsize + len;
>>> +
>>> + /* This would be a bug */
>>> + BUG_ON(dsize >= count);
>>
>> powerpc is not ready to handle BUG_ON() in when in prom_init.
>>
>> Can you do:
>>
>> #ifndef __EARLY_STRING_ENABLED
>> BUG_ON(dsize >= count);
>> #endif
>
> In fact, should be like in prom_init today:
>
> #ifdef __EARLY_STRING_ENABLED
> if (dsize >= count)
> return count;
> #else
> BUG_ON(dsize >= count);
> #endif
Thinking about it once more, this BUG_ON() is overkill and should be avoided, see
https://www.kernel.org/doc/html/latest/process/deprecated.html
Therefore, something like the following would make it:
if (dsize >= count) {
WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));
return count;
}
>
>>
>>
>>> +
>>> + dest += dsize;
>>> + count -= dsize;
>>> + if (len >= count)
>>> + len = count-1;
>>> + memcpy(dest, src, len);
>>> + dest[len] = 0;
>>> + return res;
>>> +}
>>> +EXPORT_SYMBOL(strlcat);
>>> +#endif
>>> +
^ permalink raw reply
* [PATCH RESEND] iov_iter: Use user_read_access_begin() instead of user_access_begin()
From: Christophe Leroy @ 2021-05-01 8:15 UTC (permalink / raw)
To: Alexander Viro, Andrew Morton; +Cc: linux-mm, linuxppc-dev, linux-kernel
copy_compat_iovec_from_user() only do unsafe_get_user(),
it only requires read access.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
Resending with mm list in addition
lib/iov_iter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 61228a6c69f8..fd9bd229d51d 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1932,7 +1932,7 @@ static int copy_compat_iovec_from_user(struct iovec *iov,
(const struct compat_iovec __user *)uvec;
int ret = -EFAULT, i;
- if (!user_access_begin(uiov, nr_segs * sizeof(*uiov)))
+ if (!user_read_access_begin(uiov, nr_segs * sizeof(*uiov)))
return -EFAULT;
for (i = 0; i < nr_segs; i++) {
@@ -1953,7 +1953,7 @@ static int copy_compat_iovec_from_user(struct iovec *iov,
ret = 0;
uaccess_end:
- user_access_end();
+ user_read_access_end();
return ret;
}
--
2.25.0
^ permalink raw reply related
* [PATCH] Raise the minimum GCC version to 5.2
From: Masahiro Yamada @ 2021-05-01 15:15 UTC (permalink / raw)
To: linux-kernel, Linus Torvalds
Cc: Albert Ou, Arnd Bergmann, linux-kbuild, Greg Kroah-Hartman,
Masahiro Yamada, Jonathan Corbet, linux-doc, linuxppc-dev,
Palmer Dabbelt, Paul Walmsley, Catalin Marinas, Miguel Ojeda,
Paul Mackerras, linux-riscv, Will Deacon, linux-arm-kernel
The current minimum GCC version is 4.9 except ARCH=arm64 requiring
GCC 5.1.
When we discussed last time, we agreed to raise the minimum GCC version
to 5.1 globally. [1]
I'd like to propose GCC 5.2 to clean up arch/powerpc/Kconfig as well.
This commit updates the minimum versions in scripts/min-tool-version.sh
and Documentation/process/changes.rst with trivial code cleanups.
More cleanups will be possible as follow-up patches, but this one must
be agreed and applied to the mainline first.
[1]: https://lore.kernel.org/lkml/CAHk-=wjHTpG+gMx9vqrZgo8Uw0NqA2kNjS87o63Zv3=WG2K3zA@mail.gmail.com/
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
I'd like Linus to pick up this patch
if there is no objection.
Documentation/process/changes.rst | 2 +-
arch/arm64/Kconfig | 2 +-
arch/powerpc/Kconfig | 2 +-
arch/riscv/Kconfig | 2 +-
include/linux/compiler-gcc.h | 6 +-----
lib/Kconfig.debug | 2 +-
scripts/min-tool-version.sh | 8 +-------
7 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index dac17711dc11..cf104a8d1850 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -29,7 +29,7 @@ you probably needn't concern yourself with pcmciautils.
====================== =============== ========================================
Program Minimal version Command to check the version
====================== =============== ========================================
-GNU C 4.9 gcc --version
+GNU C 5.2 gcc --version
Clang/LLVM (optional) 10.0.1 clang --version
GNU make 3.81 make --version
binutils 2.23 ld -v
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7f2a80091337..fae9514dabab 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -78,7 +78,7 @@ config ARM64
select ARCH_SUPPORTS_LTO_CLANG_THIN
select ARCH_SUPPORTS_CFI_CLANG
select ARCH_SUPPORTS_ATOMIC_RMW
- select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && (GCC_VERSION >= 50000 || CC_IS_CLANG)
+ select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
select ARCH_SUPPORTS_NUMA_BALANCING
select ARCH_WANT_COMPAT_IPC_PARSE_VERSION if COMPAT
select ARCH_WANT_DEFAULT_BPF_JIT
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1e6230bea09d..10dc47eac122 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -212,7 +212,7 @@ config PPC
select HAVE_FUNCTION_ERROR_INJECTION
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER
- select HAVE_GCC_PLUGINS if GCC_VERSION >= 50200 # plugin support on gcc <= 5.1 is buggy on PPC
+ select HAVE_GCC_PLUGINS
select HAVE_GENERIC_VDSO
select HAVE_HW_BREAKPOINT if PERF_EVENTS && (PPC_BOOK3S || PPC_8xx)
select HAVE_IDE
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4515a10c5d22..748a5b37a0e5 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -226,7 +226,7 @@ config ARCH_RV32I
config ARCH_RV64I
bool "RV64I"
select 64BIT
- select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000
+ select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_GCC
select HAVE_DYNAMIC_FTRACE if MMU
select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
select HAVE_FTRACE_MCOUNT_RECORD
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 5d97ef738a57..3608189dfc29 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -98,10 +98,8 @@
#if GCC_VERSION >= 70000
#define KASAN_ABI_VERSION 5
-#elif GCC_VERSION >= 50000
+#else
#define KASAN_ABI_VERSION 4
-#elif GCC_VERSION >= 40902
-#define KASAN_ABI_VERSION 3
#endif
#if __has_attribute(__no_sanitize_address__)
@@ -122,9 +120,7 @@
#define __no_sanitize_undefined
#endif
-#if GCC_VERSION >= 50100
#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
/*
* Turn individual warnings and errors on and off locally, depending
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 678c13967580..0d0ed298905d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -284,7 +284,7 @@ config DEBUG_INFO_DWARF4
config DEBUG_INFO_DWARF5
bool "Generate DWARF Version 5 debuginfo"
- depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
+ depends on CC_IS_GCC || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
depends on !DEBUG_INFO_BTF
help
Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ (gcc
diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index d22cf91212b0..d5d0d26b8e7d 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -17,13 +17,7 @@ binutils)
echo 2.23.0
;;
gcc)
- # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
- # https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
- if [ "$SRCARCH" = arm64 ]; then
- echo 5.1.0
- else
- echo 4.9.0
- fi
+ echo 5.2.0
;;
icc)
# temporary
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/memtrace: Fix dcache flushing
From: Sandipan Das @ 2021-05-01 15:31 UTC (permalink / raw)
To: Christophe Leroy, mpe
Cc: aneesh.kumar, rashmica.g, jniethe5, linuxppc-dev, david
In-Reply-To: <cb700884-b9f3-f3b9-4fae-272d678d1626@csgroup.eu>
On 30/04/21 2:12 pm, Christophe Leroy wrote:
>
>
> Le 30/04/2021 à 09:55, Sandipan Das a écrit :
>> Trace memory is cleared and the corresponding dcache lines
>> are flushed after allocation. However, this should not be
>> done using the PFN. This adds the missing __va() conversion.
>>
>> Fixes: 2ac02e5ecec0 ("powerpc/mm: Remove dcache flush from memory remove.")
>> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/memtrace.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
>> index 71c1262589fe..a31f13814f2e 100644
>> --- a/arch/powerpc/platforms/powernv/memtrace.c
>> +++ b/arch/powerpc/platforms/powernv/memtrace.c
>> @@ -104,8 +104,8 @@ static void memtrace_clear_range(unsigned long start_pfn,
>> * Before we go ahead and use this range as cache inhibited range
>> * flush the cache.
>> */
>> - flush_dcache_range_chunked(PFN_PHYS(start_pfn),
>> - PFN_PHYS(start_pfn + nr_pages),
>> + flush_dcache_range_chunked((unsigned long)__va(PFN_PHYS(start_pfn)),
>> + (unsigned long)__va(PFN_PHYS(start_pfn + nr_pages)),
>
> Can you use pfn_to_virt() instead ?
>
>> FLUSH_CHUNK_SIZE);
>> }
>>
Sure, that looks cleaner. Thanks for the suggestion.
- Sandipan
^ permalink raw reply
* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Miguel Ojeda @ 2021-05-01 15:52 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Albert Ou, Arnd Bergmann, Linux Kbuild mailing list,
Greg Kroah-Hartman, Jonathan Corbet, Linux Doc Mailing List,
linux-kernel, Linus Torvalds, Palmer Dabbelt, Paul Walmsley,
Catalin Marinas, Miguel Ojeda, Paul Mackerras, linux-riscv,
linuxppc-dev, Will Deacon, Linux ARM
In-Reply-To: <20210501151538.145449-1-masahiroy@kernel.org>
On Sat, May 1, 2021 at 5:17 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> More cleanups will be possible as follow-up patches, but this one must
> be agreed and applied to the mainline first.
+1 This will allow me to remove the __has_attribute hack in
include/linux/compiler_attributes.h.
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
^ permalink raw reply
* [PATCH v2] powerpc/powernv/memtrace: Fix dcache flushing
From: Sandipan Das @ 2021-05-01 16:02 UTC (permalink / raw)
To: mpe; +Cc: david, aneesh.kumar, jniethe5, rashmica.g, linuxppc-dev
Trace memory is cleared and the corresponding dcache lines
are flushed after allocation. However, this should not be
done using the PFN. This adds the missing conversion to
virtual address.
Fixes: 2ac02e5ecec0 ("powerpc/mm: Remove dcache flush from memory remove.")
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
Previous versions can be found at:
v1: https://lore.kernel.org/linuxppc-dev/20210430075557.893819-1-sandipan@linux.ibm.com/
Changes in v2:
- Added review tag from Aneesh.
- Used pfn_to_kaddr() based on Christophe's suggestion.
---
arch/powerpc/platforms/powernv/memtrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index 71c1262589fe..537a4daed614 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -104,8 +104,8 @@ static void memtrace_clear_range(unsigned long start_pfn,
* Before we go ahead and use this range as cache inhibited range
* flush the cache.
*/
- flush_dcache_range_chunked(PFN_PHYS(start_pfn),
- PFN_PHYS(start_pfn + nr_pages),
+ flush_dcache_range_chunked((unsigned long)pfn_to_kaddr(start_pfn),
+ (unsigned long)pfn_to_kaddr(start_pfn + nr_pages),
FLUSH_CHUNK_SIZE);
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Christophe Leroy @ 2021-05-01 17:38 UTC (permalink / raw)
To: Miguel Ojeda, Masahiro Yamada
Cc: Albert Ou, Catalin Marinas, Arnd Bergmann, Jonathan Corbet,
Greg Kroah-Hartman, Linux Doc Mailing List, linux-kernel,
linuxppc-dev, Will Deacon, Palmer Dabbelt, Linux ARM,
Paul Walmsley, Miguel Ojeda, Paul Mackerras, linux-riscv,
Linus Torvalds, Linux Kbuild mailing list
In-Reply-To: <CANiq72k1hB3X6+Nc_iu=f=BoB-F9JW2j_B4ZMcv8_UpW5QQ2Og@mail.gmail.com>
Le 01/05/2021 à 17:52, Miguel Ojeda a écrit :
> On Sat, May 1, 2021 at 5:17 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> More cleanups will be possible as follow-up patches, but this one must
>> be agreed and applied to the mainline first.
>
> +1 This will allow me to remove the __has_attribute hack in
> include/linux/compiler_attributes.h.
>
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
>
On powerpc this will allow us to remove commit https://github.com/linuxppc/linux/commit/592bbe9c505d
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Christophe
^ permalink raw reply
* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Joe Perches @ 2021-05-02 2:41 UTC (permalink / raw)
To: Miguel Ojeda, Masahiro Yamada
Cc: Albert Ou, Arnd Bergmann, Linux Kbuild mailing list,
Greg Kroah-Hartman, Jonathan Corbet, Linux Doc Mailing List,
linux-kernel, Linus Torvalds, Palmer Dabbelt, Paul Walmsley,
Catalin Marinas, Miguel Ojeda, Paul Mackerras, linux-riscv,
linuxppc-dev, Will Deacon, Linux ARM
In-Reply-To: <CANiq72k1hB3X6+Nc_iu=f=BoB-F9JW2j_B4ZMcv8_UpW5QQ2Og@mail.gmail.com>
On Sat, 2021-05-01 at 17:52 +0200, Miguel Ojeda wrote:
> On Sat, May 1, 2021 at 5:17 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > More cleanups will be possible as follow-up patches, but this one must
> > be agreed and applied to the mainline first.
>
> +1 This will allow me to remove the __has_attribute hack in
> include/linux/compiler_attributes.h.
Why not raise the minimum gcc compiler version even higher?
https://gcc.gnu.org/releases.html
^ permalink raw reply
* [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
From: Nicholas Piggin @ 2021-05-02 4:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
This reduces TLB misses by nearly 30x on a `git diff` workload on a
2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
to vfs hashes being allocated with 2MB pages.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
This was in the -mm tree but was dropped at the last minute after
clashing with a patch in powerpc next.
Now all prerequisites are upstream, this can be merged as is. Probably
makes sense now to go via powerpc tree.
This is rebased and retested on upstream.
Documentation/admin-guide/kernel-parameters.txt | 2 ++
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/pgtable.h | 5 +++++
arch/powerpc/kernel/module.c | 16 +++++++++++++---
4 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1c0a3cf6fcc9..1be38b25c485 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3250,6 +3250,8 @@
nohugeiomap [KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
+ nohugevmalloc [PPC] Disable kernel huge vmalloc mappings.
+
nosmt [KNL,S390] Disable symmetric multithreading (SMT).
Equivalent to smt=1.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1e6230bea09d..c547a9d6a2dd 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -185,6 +185,7 @@ config PPC
select GENERIC_VDSO_TIME_NS
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
+ select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index c6a676714f04..1678e4b08fc3 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -39,6 +39,11 @@ struct mm_struct;
#define __S110 PAGE_SHARED_X
#define __S111 PAGE_SHARED_X
+#ifndef MODULES_VADDR
+#define MODULES_VADDR VMALLOC_START
+#define MODULES_END VMALLOC_END
+#endif
+
#ifndef __ASSEMBLY__
/* Keep these as a macros to avoid include dependency mess */
diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index fab84024650c..77aefcbbd276 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -8,6 +8,7 @@
#include <linux/moduleloader.h>
#include <linux/err.h>
#include <linux/vmalloc.h>
+#include <linux/mm.h>
#include <linux/bug.h>
#include <asm/module.h>
#include <linux/uaccess.h>
@@ -88,17 +89,24 @@ int module_finalize(const Elf_Ehdr *hdr,
return 0;
}
-#ifdef MODULES_VADDR
static __always_inline void *
__module_alloc(unsigned long size, unsigned long start, unsigned long end)
{
+ /*
+ * Don't do huge page allocations for modules yet until more testing
+ * is done. STRICT_MODULE_RWX may require extra work to support this
+ * too.
+ */
return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
- PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
+ PAGE_KERNEL_EXEC,
+ VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
+ NUMA_NO_NODE,
__builtin_return_address(0));
}
void *module_alloc(unsigned long size)
{
+#ifdef CONFIG_PPC32
unsigned long limit = (unsigned long)_etext - SZ_32M;
void *ptr = NULL;
@@ -112,5 +120,7 @@ void *module_alloc(unsigned long size)
ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
return ptr;
-}
+#else
+ return __module_alloc(size, MODULES_VADDR, MODULES_END);
#endif
+}
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
From: Christophe Leroy @ 2021-05-02 7:34 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20210502045615.237268-1-npiggin@gmail.com>
Le 02/05/2021 à 06:56, Nicholas Piggin a écrit :
> This reduces TLB misses by nearly 30x on a `git diff` workload on a
> 2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
> to vfs hashes being allocated with 2MB pages.
>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> This was in the -mm tree but was dropped at the last minute after
> clashing with a patch in powerpc next.
>
> Now all prerequisites are upstream, this can be merged as is. Probably
> makes sense now to go via powerpc tree.
>
> This is rebased and retested on upstream.
>
> Documentation/admin-guide/kernel-parameters.txt | 2 ++
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/pgtable.h | 5 +++++
> arch/powerpc/kernel/module.c | 16 +++++++++++++---
> 4 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1c0a3cf6fcc9..1be38b25c485 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3250,6 +3250,8 @@
>
> nohugeiomap [KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
>
> + nohugevmalloc [PPC] Disable kernel huge vmalloc mappings.
> +
> nosmt [KNL,S390] Disable symmetric multithreading (SMT).
> Equivalent to smt=1.
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 1e6230bea09d..c547a9d6a2dd 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -185,6 +185,7 @@ config PPC
> select GENERIC_VDSO_TIME_NS
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
> + select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
> select HAVE_ARCH_JUMP_LABEL
> select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index c6a676714f04..1678e4b08fc3 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -39,6 +39,11 @@ struct mm_struct;
> #define __S110 PAGE_SHARED_X
> #define __S111 PAGE_SHARED_X
>
> +#ifndef MODULES_VADDR
> +#define MODULES_VADDR VMALLOC_START
> +#define MODULES_END VMALLOC_END
> +#endif
This will also require some changes in a few places, see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210429031602.2606654-4-jniethe5@gmail.com/
> +
> #ifndef __ASSEMBLY__
>
> /* Keep these as a macros to avoid include dependency mess */
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index fab84024650c..77aefcbbd276 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -8,6 +8,7 @@
> #include <linux/moduleloader.h>
> #include <linux/err.h>
> #include <linux/vmalloc.h>
> +#include <linux/mm.h>
> #include <linux/bug.h>
> #include <asm/module.h>
> #include <linux/uaccess.h>
> @@ -88,17 +89,24 @@ int module_finalize(const Elf_Ehdr *hdr,
> return 0;
> }
>
> -#ifdef MODULES_VADDR
> static __always_inline void *
> __module_alloc(unsigned long size, unsigned long start, unsigned long end)
> {
> + /*
> + * Don't do huge page allocations for modules yet until more testing
> + * is done. STRICT_MODULE_RWX may require extra work to support this
> + * too.
> + */
> return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> + PAGE_KERNEL_EXEC,
> + VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
> + NUMA_NO_NODE,
> __builtin_return_address(0));
Can we avoid so many lines ? Doesn't it fit on 3 lines now that 100 chars per line are tolerated ?
> }
>
> void *module_alloc(unsigned long size)
> {
> +#ifdef CONFIG_PPC32
Can we just add an IS_ENABLED(CONFIG_PPC32) in the 'if' instead of this #ifdef/#else ?
> unsigned long limit = (unsigned long)_etext - SZ_32M;
> void *ptr = NULL;
>
> @@ -112,5 +120,7 @@ void *module_alloc(unsigned long size)
> ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>
> return ptr;
> -}
> +#else
> + return __module_alloc(size, MODULES_VADDR, MODULES_END);
> #endif
> +}
>
^ permalink raw reply
* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
From: kernel test robot @ 2021-05-02 8:03 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin
In-Reply-To: <20210502045615.237268-1-npiggin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]
Hi Nicholas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master next-20210430]
[cannot apply to scottwood/next v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1c8b90cff6679d7690891101efc07a1edd98e9a7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
git checkout 1c8b90cff6679d7690891101efc07a1edd98e9a7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/proc/kcore.c: In function 'add_modules_range':
>> fs/proc/kcore.c:626:20: warning: self-comparison always evaluates to false [-Wtautological-compare]
626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
| ^~
fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to false [-Wtautological-compare]
626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
| ^~
vim +626 fs/proc/kcore.c
9492587cf35d37 KAMEZAWA Hiroyuki 2009-09-22 618
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 619 #if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 620 /*
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 621 * MODULES_VADDR has no intersection with VMALLOC_ADDR.
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 622 */
eebf36480678f9 YueHaibing 2019-03-28 623 static struct kcore_list kcore_modules;
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 624 static void __init add_modules_range(void)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 625 {
bf3e2692468fe4 Baoquan He 2014-10-09 @626 if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 627 kclist_add(&kcore_modules, (void *)MODULES_VADDR,
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 628 MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 629 }
bf3e2692468fe4 Baoquan He 2014-10-09 630 }
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 631 #else
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 632 static void __init add_modules_range(void)
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 633 {
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 634 }
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 635 #endif
81ac3ad9061dd9 KAMEZAWA Hiroyuki 2009-09-22 636
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26774 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
From: kernel test robot @ 2021-05-02 8:03 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin
In-Reply-To: <20210502045615.237268-1-npiggin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3162 bytes --]
Hi Nicholas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master next-20210430]
[cannot apply to v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r013-20210502 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1c8b90cff6679d7690891101efc07a1edd98e9a7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-125808
git checkout 1c8b90cff6679d7690891101efc07a1edd98e9a7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/nohash/mmu.h:16,
from arch/powerpc/include/asm/mmu.h:423,
from arch/powerpc/kernel/head_8xx.S:23:
>> arch/powerpc/include/asm/nohash/32/mmu-8xx.h:175: warning: "MODULES_VADDR" redefined
175 | #define MODULES_VADDR (PAGE_OFFSET - SZ_256M)
|
In file included from include/linux/pgtable.h:6,
from arch/powerpc/kernel/head_8xx.S:19:
arch/powerpc/include/asm/pgtable.h:43: note: this is the location of the previous definition
43 | #define MODULES_VADDR VMALLOC_START
|
In file included from arch/powerpc/include/asm/nohash/mmu.h:16,
from arch/powerpc/include/asm/mmu.h:423,
from arch/powerpc/kernel/head_8xx.S:23:
>> arch/powerpc/include/asm/nohash/32/mmu-8xx.h:176: warning: "MODULES_END" redefined
176 | #define MODULES_END PAGE_OFFSET
|
In file included from include/linux/pgtable.h:6,
from arch/powerpc/kernel/head_8xx.S:19:
arch/powerpc/include/asm/pgtable.h:44: note: this is the location of the previous definition
44 | #define MODULES_END VMALLOC_END
|
vim +/MODULES_VADDR +175 arch/powerpc/include/asm/nohash/32/mmu-8xx.h
fca5c1e9eb5e26 Christophe Leroy 2019-04-25 174
9132a2e82adc6e Christophe Leroy 2021-04-01 @175 #define MODULES_VADDR (PAGE_OFFSET - SZ_256M)
9132a2e82adc6e Christophe Leroy 2021-04-01 @176 #define MODULES_END PAGE_OFFSET
9132a2e82adc6e Christophe Leroy 2021-04-01 177
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38771 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/64s/radix: Enable huge vmalloc mappings
From: Nicholas Piggin @ 2021-05-02 10:41 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <60e4f5d6-67ae-42a9-5edd-bed2dfde2eb3@csgroup.eu>
Excerpts from Christophe Leroy's message of May 2, 2021 5:34 pm:
>
>
> Le 02/05/2021 à 06:56, Nicholas Piggin a écrit :
>> This reduces TLB misses by nearly 30x on a `git diff` workload on a
>> 2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
>> to vfs hashes being allocated with 2MB pages.
>>
>> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> This was in the -mm tree but was dropped at the last minute after
>> clashing with a patch in powerpc next.
>>
>> Now all prerequisites are upstream, this can be merged as is. Probably
>> makes sense now to go via powerpc tree.
>>
>> This is rebased and retested on upstream.
>>
>> Documentation/admin-guide/kernel-parameters.txt | 2 ++
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/include/asm/pgtable.h | 5 +++++
>> arch/powerpc/kernel/module.c | 16 +++++++++++++---
>> 4 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 1c0a3cf6fcc9..1be38b25c485 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3250,6 +3250,8 @@
>>
>> nohugeiomap [KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
>>
>> + nohugevmalloc [PPC] Disable kernel huge vmalloc mappings.
>> +
>> nosmt [KNL,S390] Disable symmetric multithreading (SMT).
>> Equivalent to smt=1.
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 1e6230bea09d..c547a9d6a2dd 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -185,6 +185,7 @@ config PPC
>> select GENERIC_VDSO_TIME_NS
>> select HAVE_ARCH_AUDITSYSCALL
>> select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
>> + select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
>> select HAVE_ARCH_JUMP_LABEL
>> select HAVE_ARCH_JUMP_LABEL_RELATIVE
>> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
>> index c6a676714f04..1678e4b08fc3 100644
>> --- a/arch/powerpc/include/asm/pgtable.h
>> +++ b/arch/powerpc/include/asm/pgtable.h
>> @@ -39,6 +39,11 @@ struct mm_struct;
>> #define __S110 PAGE_SHARED_X
>> #define __S111 PAGE_SHARED_X
>>
>> +#ifndef MODULES_VADDR
>> +#define MODULES_VADDR VMALLOC_START
>> +#define MODULES_END VMALLOC_END
>> +#endif
>
> This will also require some changes in a few places, see
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210429031602.2606654-4-jniethe5@gmail.com/
I see.
I'll just make the PPC64 version use VMALLOC_START/VMALLOC_END, which
avoids that stupid compiler warning found by kbuild robot as well.
>
>> +
>> #ifndef __ASSEMBLY__
>>
>> /* Keep these as a macros to avoid include dependency mess */
>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
>> index fab84024650c..77aefcbbd276 100644
>> --- a/arch/powerpc/kernel/module.c
>> +++ b/arch/powerpc/kernel/module.c
>> @@ -8,6 +8,7 @@
>> #include <linux/moduleloader.h>
>> #include <linux/err.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/mm.h>
>> #include <linux/bug.h>
>> #include <asm/module.h>
>> #include <linux/uaccess.h>
>> @@ -88,17 +89,24 @@ int module_finalize(const Elf_Ehdr *hdr,
>> return 0;
>> }
>>
>> -#ifdef MODULES_VADDR
>> static __always_inline void *
>> __module_alloc(unsigned long size, unsigned long start, unsigned long end)
>> {
>> + /*
>> + * Don't do huge page allocations for modules yet until more testing
>> + * is done. STRICT_MODULE_RWX may require extra work to support this
>> + * too.
>> + */
>> return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
>> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>> + PAGE_KERNEL_EXEC,
>> + VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
>> + NUMA_NO_NODE,
>> __builtin_return_address(0));
>
> Can we avoid so many lines ? Doesn't it fit on 3 lines now that 100 chars per line are tolerated ?
It could be squashed onto fewer lines. Is it better?
>
>> }
>>
>> void *module_alloc(unsigned long size)
>> {
>> +#ifdef CONFIG_PPC32
>
> Can we just add an IS_ENABLED(CONFIG_PPC32) in the 'if' instead of this #ifdef/#else ?
I guess not if I don't define MODULES_VADDR. Jordan's clenup patch could
change it to IS_ENABLED.
Thanks,
Nick
>
>> unsigned long limit = (unsigned long)_etext - SZ_32M;
>> void *ptr = NULL;
>>
>> @@ -112,5 +120,7 @@ void *module_alloc(unsigned long size)
>> ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>>
>> return ptr;
>> -}
>> +#else
>> + return __module_alloc(size, MODULES_VADDR, MODULES_END);
>> #endif
>> +}
>>
>
^ permalink raw reply
* [PATCH v2] powerpc/64s/radix: Enable huge vmalloc mappings
From: Nicholas Piggin @ 2021-05-02 11:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
This reduces TLB misses by nearly 30x on a `git diff` workload on a
2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
to vfs hashes being allocated with 2MB pages.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since v1:
- Don't define MODULES_VADDR which has some other side effect (e.g.,
ptdump).
- Fixed (hopefully) kbuild warning.
- Keep __vmalloc_node_range call on 3 lines.
.../admin-guide/kernel-parameters.txt | 2 ++
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/module.c | 18 +++++++++++++-----
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1c0a3cf6fcc9..1be38b25c485 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3250,6 +3250,8 @@
nohugeiomap [KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
+ nohugevmalloc [PPC] Disable kernel huge vmalloc mappings.
+
nosmt [KNL,S390] Disable symmetric multithreading (SMT).
Equivalent to smt=1.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1e6230bea09d..c547a9d6a2dd 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -185,6 +185,7 @@ config PPC
select GENERIC_VDSO_TIME_NS
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
+ select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index fab84024650c..ea1fa55a6897 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -8,6 +8,7 @@
#include <linux/moduleloader.h>
#include <linux/err.h>
#include <linux/vmalloc.h>
+#include <linux/mm.h>
#include <linux/bug.h>
#include <asm/module.h>
#include <linux/uaccess.h>
@@ -88,17 +89,22 @@ int module_finalize(const Elf_Ehdr *hdr,
return 0;
}
-#ifdef MODULES_VADDR
static __always_inline void *
__module_alloc(unsigned long size, unsigned long start, unsigned long end)
{
- return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
- PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
- __builtin_return_address(0));
+ /*
+ * Don't do huge page allocations for modules yet until more testing
+ * is done. STRICT_MODULE_RWX may require extra work to support this
+ * too.
+ */
+ return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, PAGE_KERNEL_EXEC,
+ VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
+ NUMA_NO_NODE, __builtin_return_address(0));
}
void *module_alloc(unsigned long size)
{
+#ifdef CONFIG_PPC32
unsigned long limit = (unsigned long)_etext - SZ_32M;
void *ptr = NULL;
@@ -112,5 +118,7 @@ void *module_alloc(unsigned long size)
ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
return ptr;
-}
+#else
+ return __module_alloc(size, VMALLOC_START, VMALLOC_END);
#endif
+}
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v2] powerpc/64s/radix: Enable huge vmalloc mappings
From: kernel test robot @ 2021-05-02 13:12 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin
In-Reply-To: <20210502110050.324953-1-npiggin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4407 bytes --]
Hi Nicholas,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on linus/master next-20210430]
[cannot apply to scottwood/next v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-190249
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-akebono_defconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/897024543f33f779ce158849e73999b9d1271f57
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-radix-Enable-huge-vmalloc-mappings/20210502-190249
git checkout 897024543f33f779ce158849e73999b9d1271f57
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/powerpc/kernel/module.c: In function '__module_alloc':
arch/powerpc/kernel/module.c:101:32: error: 'VM_NO_HUGE_VMAP' undeclared (first use in this function)
101 | VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
| ^~~~~~~~~~~~~~~
arch/powerpc/kernel/module.c:101:32: note: each undeclared identifier is reported only once for each function it appears in
In file included from <command-line>:
arch/powerpc/kernel/module.c: In function 'module_alloc':
>> arch/powerpc/kernel/module.c:111:27: error: 'MODULES_VADDR' undeclared (first use in this function)
111 | BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
| ^~~~~~~~~~~~~
include/linux/compiler_types.h:300:9: note: in definition of macro '__compiletime_assert'
300 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:320:2: note: in expansion of macro '_compiletime_assert'
320 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^~~~~~~~~~~~~~~~
arch/powerpc/kernel/module.c:111:2: note: in expansion of macro 'BUILD_BUG_ON'
111 | BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
| ^~~~~~~~~~~~
>> arch/powerpc/kernel/module.c:114:37: error: 'MODULES_END' undeclared (first use in this function)
114 | if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
| ^~~~~~~~~~~
vim +/MODULES_VADDR +111 arch/powerpc/kernel/module.c
2ec13df167040c Christophe Leroy 2021-04-01 104
7fbc22ce299316 Christophe Leroy 2020-06-29 105 void *module_alloc(unsigned long size)
7fbc22ce299316 Christophe Leroy 2020-06-29 106 {
897024543f33f7 Nicholas Piggin 2021-05-02 107 #ifdef CONFIG_PPC32
2ec13df167040c Christophe Leroy 2021-04-01 108 unsigned long limit = (unsigned long)_etext - SZ_32M;
2ec13df167040c Christophe Leroy 2021-04-01 109 void *ptr = NULL;
2ec13df167040c Christophe Leroy 2021-04-01 110
7fbc22ce299316 Christophe Leroy 2020-06-29 @111 BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
7fbc22ce299316 Christophe Leroy 2020-06-29 112
2ec13df167040c Christophe Leroy 2021-04-01 113 /* First try within 32M limit from _etext to avoid branch trampolines */
2ec13df167040c Christophe Leroy 2021-04-01 @114 if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17525 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/pseries: Fix hcall tracing recursion in pv queued spinlocks
From: Michael Ellerman @ 2021-05-02 13:48 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, Naveen N. Rao
In-Reply-To: <1619832010.xbqdcxufia.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Naveen N. Rao's message of April 27, 2021 11:43 pm:
>> Nicholas Piggin wrote:
>>> The paravit queued spinlock slow path adds itself to the queue then
>>> calls pv_wait to wait for the lock to become free. This is implemented
>>> by calling H_CONFER to donate cycles.
>>>
>>> When hcall tracing is enabled, this H_CONFER call can lead to a spin
>>> lock being taken in the tracing code, which will result in the lock to
>>> be taken again, which will also go to the slow path because it queues
>>> behind itself and so won't ever make progress.
>>>
>>> An example trace of a deadlock:
>>>
>>> __pv_queued_spin_lock_slowpath
>>> trace_clock_global
>>> ring_buffer_lock_reserve
>>> trace_event_buffer_lock_reserve
>>> trace_event_buffer_reserve
>>> trace_event_raw_event_hcall_exit
>>> __trace_hcall_exit
>>> plpar_hcall_norets_trace
>>> __pv_queued_spin_lock_slowpath
>>> trace_clock_global
>>> ring_buffer_lock_reserve
>>> trace_event_buffer_lock_reserve
>>> trace_event_buffer_reserve
>>> trace_event_raw_event_rcu_dyntick
>>> rcu_irq_exit
>>> irq_exit
>>> __do_irq
>>> call_do_irq
>>> do_IRQ
>>> hardware_interrupt_common_virt
>>>
>>> Fix this by introducing plpar_hcall_norets_notrace(), and using that to
>>> make SPLPAR virtual processor dispatching hcalls by the paravirt
>>> spinlock code.
>>>
>>> Fixes: 20c0e8269e9d ("powerpc/pseries: Implement paravirt qspinlocks for SPLPAR")
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>> arch/powerpc/include/asm/hvcall.h | 3 +++
>>> arch/powerpc/include/asm/paravirt.h | 22 +++++++++++++++++++---
>>> arch/powerpc/platforms/pseries/hvCall.S | 10 ++++++++++
>>> arch/powerpc/platforms/pseries/lpar.c | 4 ++--
>>> 4 files changed, 34 insertions(+), 5 deletions(-)
>>
>> Thanks for the fix! Some very minor nits below, but none the less:
>> Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>
>>>
>>> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
>>> index ed6086d57b22..0c92b01a3c3c 100644
>>> --- a/arch/powerpc/include/asm/hvcall.h
>>> +++ b/arch/powerpc/include/asm/hvcall.h
>>> @@ -446,6 +446,9 @@
>>> */
>>> long plpar_hcall_norets(unsigned long opcode, ...);
>>>
>>> +/* Variant which does not do hcall tracing */
>>> +long plpar_hcall_norets_notrace(unsigned long opcode, ...);
>>> +
>>> /**
>>> * plpar_hcall: - Make a pseries hypervisor call
>>> * @opcode: The hypervisor call to make.
>>> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
>>> index 5d1726bb28e7..3c13c2ec70a9 100644
>>> --- a/arch/powerpc/include/asm/paravirt.h
>>> +++ b/arch/powerpc/include/asm/paravirt.h
>>> @@ -30,17 +30,33 @@ static inline u32 yield_count_of(int cpu)
>>>
>>> static inline void yield_to_preempted(int cpu, u32 yield_count)
>>> {
>>
>> It looks like yield_to_preempted() is only used by simple spin locks
>> today. I wonder if it makes more sense to put the below comment in
>> yield_to_any() which is used by the qspinlock code.
>
> Yeah, I just put it above the functions entirely because it refers to
> all of them.
>
>>
>>> - plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_count);
>>> + /*
>>> + * Spinlock code yields and prods, so don't trace the hcalls because
>>> + * tracing code takes spinlocks which could recurse.
>>> + *
>>> + * These calls are made while the lock is not held, the lock slowpath
>>> + * yields if it can not acquire the lock, and unlock slow path might
>>> + * prod if a waiter has yielded). So this did not seem to be a problem
>>> + * for simple spin locks because technically it didn't recuse on the
>> ^^^^^^
>> recurse
>>
>>> + * lock. However the queued spin lock contended path is more strictly
>>> + * ordered: the H_CONFER hcall is made after the task has queued itself
>>> + * on the lock, so then recursing on the lock will queue up behind that
>>> + * (or worse: queued spinlocks uses tricks that assume a context never
>>> + * waits on more than one spinlock, so that may cause random
>>> + * corruption).
>>> + */
>>> + plpar_hcall_norets_notrace(H_CONFER,
>>> + get_hard_smp_processor_id(cpu), yield_count);
>>
>> This can all be on a single line.
>
> Should it though? Linux in general allegedly changed to 100 column
> lines for checkpatch, but it seems to still be frowned upon to go
> beyond 80 deliberately. What about arch/powerpc?
Splitting it provides zero benefit to code readability IMO. And it would
be only 89 by my count, which is not grossly long.
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Michal Suchánek @ 2021-05-02 16:57 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200428112517.1402927-1-npiggin@gmail.com>
On Tue, Apr 28, 2020 at 09:25:17PM +1000, Nicholas Piggin wrote:
> Provide an option to use ELFv2 ABI for big endian builds. This works on
> GCC and clang (since 2014). It is less well tested and supported by the
> GNU toolchain, but it can give some useful advantages of the ELFv2 ABI
> for BE (e.g., less stack usage). Some distros even build BE ELFv2
> userspace.
Fixes BTFID failure on BE for me and the ELF ABIv2 kernel boots.
Tested-by: Michal Suchánek <msuchanek@suse.de>
Also can we enable mprofile on BE now?
I don't see anything endian-specific in the mprofile code at a glance
but don't have any idea how to test it.
Thanks
Michal
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6a4ad11f6349..75b3afbfc378 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -495,7 +495,7 @@ config LD_HEAD_STUB_CATCH
If unsure, say "N".
config MPROFILE_KERNEL
- depends on PPC64 && CPU_LITTLE_ENDIAN && FUNCTION_TRACER
+ depends on PPC64 && BUILD_ELF_V2 && FUNCTION_TRACER
def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__)
config HOTPLUG_CPU
>
> Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Since v1:
> - Improved the override flavour name suggested by Segher.
> - Improved changelog wording.
>
>
> arch/powerpc/Kconfig | 19 +++++++++++++++++++
> arch/powerpc/Makefile | 15 ++++++++++-----
> arch/powerpc/boot/Makefile | 4 ++++
> drivers/crypto/vmx/Makefile | 8 ++++++--
> drivers/crypto/vmx/ppc-xlate.pl | 10 ++++++----
> 5 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 924c541a9260..d9d2abc06c2c 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -147,6 +147,7 @@ config PPC
> select ARCH_WEAK_RELEASE_ACQUIRE
> select BINFMT_ELF
> select BUILDTIME_TABLE_SORT
> + select BUILD_ELF_V2 if PPC64 && CPU_LITTLE_ENDIAN
> select CLONE_BACKWARDS
> select DCACHE_WORD_ACCESS if PPC64 && CPU_LITTLE_ENDIAN
> select DYNAMIC_FTRACE if FUNCTION_TRACER
> @@ -541,6 +542,24 @@ config KEXEC_FILE
> config ARCH_HAS_KEXEC_PURGATORY
> def_bool KEXEC_FILE
>
> +config BUILD_ELF_V2
> + bool
> +
> +config BUILD_BIG_ENDIAN_ELF_V2
> + bool "Build big-endian kernel using ELFv2 ABI (EXPERIMENTAL)"
> + depends on PPC64 && CPU_BIG_ENDIAN && EXPERT
> + default n
> + select BUILD_ELF_V2
> + help
> + This builds the kernel image using the ELFv2 ABI, which has a
> + reduced stack overhead and faster function calls. This does not
> + affect the userspace ABIs.
> +
> + ELFv2 is the standard ABI for little-endian, but for big-endian
> + this is an experimental option that is less tested (kernel and
> + toolchain). This requires gcc 4.9 or newer and binutils 2.24 or
> + newer.
> +
> config RELOCATABLE
> bool "Build a relocatable kernel"
> depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index f310c32e88a4..e306b39d847e 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -92,10 +92,14 @@ endif
>
> ifdef CONFIG_PPC64
> ifndef CONFIG_CC_IS_CLANG
> -cflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mabi=elfv1)
> -cflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mcall-aixdesc)
> -aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mabi=elfv1)
> -aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
> +ifdef CONFIG_BUILD_ELF_V2
> +cflags-y += $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
> +aflags-y += $(call cc-option,-mabi=elfv2)
> +else
> +cflags-y += $(call cc-option,-mabi=elfv1)
> +cflags-y += $(call cc-option,-mcall-aixdesc)
> +aflags-y += $(call cc-option,-mabi=elfv1)
> +endif
> endif
> endif
>
> @@ -144,7 +148,7 @@ endif
>
> CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
> ifndef CONFIG_CC_IS_CLANG
> -ifdef CONFIG_CPU_LITTLE_ENDIAN
> +ifdef CONFIG_BUILD_ELF_V2
> CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
> AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
> else
> @@ -153,6 +157,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
> AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
> endif
> endif
> +
> CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
> CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
>
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index c53a1b8bba8b..03942d08695d 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -41,6 +41,10 @@ endif
>
> BOOTCFLAGS += -isystem $(shell $(BOOTCC) -print-file-name=include)
>
> +ifdef CONFIG_BUILD_ELF_V2
> +BOOTCFLAGS += $(call cc-option,-mabi=elfv2)
> +endif
> +
> ifdef CONFIG_CPU_BIG_ENDIAN
> BOOTCFLAGS += -mbig-endian
> else
> diff --git a/drivers/crypto/vmx/Makefile b/drivers/crypto/vmx/Makefile
> index 709670d2b553..9aea34602beb 100644
> --- a/drivers/crypto/vmx/Makefile
> +++ b/drivers/crypto/vmx/Makefile
> @@ -5,18 +5,22 @@ vmx-crypto-objs := vmx.o aesp8-ppc.o ghashp8-ppc.o aes.o aes_cbc.o aes_ctr.o aes
> ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
> override flavour := linux-ppc64le
> else
> +ifdef CONFIG_BUILD_ELF_V2
> +override flavour := linux-ppc64-elfv2
> +else
> override flavour := linux-ppc64
> endif
> +endif
>
> quiet_cmd_perl = PERL $@
> cmd_perl = $(PERL) $(<) $(flavour) > $(@)
>
> targets += aesp8-ppc.S ghashp8-ppc.S
>
> -$(obj)/aesp8-ppc.S: $(src)/aesp8-ppc.pl FORCE
> +$(obj)/aesp8-ppc.S: $(src)/aesp8-ppc.pl $(src)/ppc-xlate.pl FORCE
> $(call if_changed,perl)
>
> -$(obj)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl FORCE
> +$(obj)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl $(src)/ppc-xlate.pl FORCE
> $(call if_changed,perl)
>
> clean-files := aesp8-ppc.S ghashp8-ppc.S
> diff --git a/drivers/crypto/vmx/ppc-xlate.pl b/drivers/crypto/vmx/ppc-xlate.pl
> index 36db2ef09e5b..9db0937d318b 100644
> --- a/drivers/crypto/vmx/ppc-xlate.pl
> +++ b/drivers/crypto/vmx/ppc-xlate.pl
> @@ -9,6 +9,8 @@ open STDOUT,">$output" || die "can't open $output: $!";
>
> my %GLOBALS;
> my $dotinlocallabels=($flavour=~/linux/)?1:0;
> +my $abielfv2=(($flavour =~ /linux-ppc64le/) or ($flavour =~ /linux-ppc64-elfv2/))?1:0;
> +my $dotfunctions=($abielfv2=~1)?0:1;
>
> ################################################################
> # directives which need special treatment on different platforms
> @@ -40,7 +42,7 @@ my $globl = sub {
> };
> my $text = sub {
> my $ret = ($flavour =~ /aix/) ? ".csect\t.text[PR],7" : ".text";
> - $ret = ".abiversion 2\n".$ret if ($flavour =~ /linux.*64le/);
> + $ret = ".abiversion 2\n".$ret if ($abielfv2);
> $ret;
> };
> my $machine = sub {
> @@ -56,8 +58,8 @@ my $size = sub {
> if ($flavour =~ /linux/)
> { shift;
> my $name = shift; $name =~ s|^[\.\_]||;
> - my $ret = ".size $name,.-".($flavour=~/64$/?".":"").$name;
> - $ret .= "\n.size .$name,.-.$name" if ($flavour=~/64$/);
> + my $ret = ".size $name,.-".($dotfunctions?".":"").$name;
> + $ret .= "\n.size .$name,.-.$name" if ($dotfunctions);
> $ret;
> }
> else
> @@ -142,7 +144,7 @@ my $vmr = sub {
>
> # Some ABIs specify vrsave, special-purpose register #256, as reserved
> # for system use.
> -my $no_vrsave = ($flavour =~ /linux-ppc64le/);
> +my $no_vrsave = ($abielfv2);
> my $mtspr = sub {
> my ($f,$idx,$ra) = @_;
> if ($idx == 256 && $no_vrsave) {
> --
> 2.23.0
>
^ permalink raw reply related
* Re: [PATCH v2] powerpc/64s/radix: Enable huge vmalloc mappings
From: Christophe Leroy @ 2021-05-02 17:11 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20210502110050.324953-1-npiggin@gmail.com>
Le 02/05/2021 à 13:00, Nicholas Piggin a écrit :
> This reduces TLB misses by nearly 30x on a `git diff` workload on a
> 2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
> to vfs hashes being allocated with 2MB pages.
>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Since v1:
> - Don't define MODULES_VADDR which has some other side effect (e.g.,
> ptdump).
> - Fixed (hopefully) kbuild warning.
> - Keep __vmalloc_node_range call on 3 lines.
>
> .../admin-guide/kernel-parameters.txt | 2 ++
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/kernel/module.c | 18 +++++++++++++-----
> 3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1c0a3cf6fcc9..1be38b25c485 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3250,6 +3250,8 @@
>
> nohugeiomap [KNL,X86,PPC,ARM64] Disable kernel huge I/O mappings.
>
> + nohugevmalloc [PPC] Disable kernel huge vmalloc mappings.
> +
> nosmt [KNL,S390] Disable symmetric multithreading (SMT).
> Equivalent to smt=1.
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 1e6230bea09d..c547a9d6a2dd 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -185,6 +185,7 @@ config PPC
> select GENERIC_VDSO_TIME_NS
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
> + select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
> select HAVE_ARCH_JUMP_LABEL
> select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index fab84024650c..ea1fa55a6897 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -8,6 +8,7 @@
> #include <linux/moduleloader.h>
> #include <linux/err.h>
> #include <linux/vmalloc.h>
> +#include <linux/mm.h>
> #include <linux/bug.h>
> #include <asm/module.h>
> #include <linux/uaccess.h>
> @@ -88,17 +89,22 @@ int module_finalize(const Elf_Ehdr *hdr,
> return 0;
> }
>
> -#ifdef MODULES_VADDR
> static __always_inline void *
> __module_alloc(unsigned long size, unsigned long start, unsigned long end)
> {
> - return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> - __builtin_return_address(0));
> + /*
> + * Don't do huge page allocations for modules yet until more testing
> + * is done. STRICT_MODULE_RWX may require extra work to support this
> + * too.
> + */
> + return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, PAGE_KERNEL_EXEC,
> + VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
> + NUMA_NO_NODE, __builtin_return_address(0));
> }
>
> void *module_alloc(unsigned long size)
> {
> +#ifdef CONFIG_PPC32
What then happens to PPC32 platforms that doesn't define MODULES_VADDR, for instance 4xx or booke ?
I think it should be:
#ifdef MODULES_VADDR
> unsigned long limit = (unsigned long)_etext - SZ_32M;
> void *ptr = NULL;
>
> @@ -112,5 +118,7 @@ void *module_alloc(unsigned long size)
> ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>
> return ptr;
> -}
> +#else
> + return __module_alloc(size, VMALLOC_START, VMALLOC_END);
> #endif
> +}
>
^ permalink raw reply
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