LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc: kernel: Trivial typo fix in the file kgdb.c
From: Michael Ellerman @ 2021-03-31  1:09 UTC (permalink / raw)
  To: linuxppc-dev, Bhaskar Chowdhury, mpe, jniethe5, benh, alistair,
	linux-kernel, paulus
  Cc: rdunlap
In-Reply-To: <20210317090413.120891-1-unixbhaskar@gmail.com>

On Wed, 17 Mar 2021 14:34:13 +0530, Bhaskar Chowdhury wrote:
> s/procesing/processing/

Applied to powerpc/next.

[1/1] powerpc: kernel: Trivial typo fix in the file kgdb.c
      https://git.kernel.org/powerpc/c/89f7d2927ae16ea470d29234447763826e40c6cf

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: mm: book3s64: Fix a typo in the file mmu_context.c
From: Michael Ellerman @ 2021-03-31  1:09 UTC (permalink / raw)
  To: npiggin, linuxppc-dev, Bhaskar Chowdhury, mpe, benh, aneesh.kumar,
	linux-kernel, paulus
  Cc: rdunlap
In-Reply-To: <20210312112537.4585-1-unixbhaskar@gmail.com>

On Fri, 12 Mar 2021 16:55:37 +0530, Bhaskar Chowdhury wrote:
> s/detalis/details/

Applied to powerpc/next.

[1/1] powerpc: mm: book3s64: Fix a typo in the file mmu_context.c
      https://git.kernel.org/powerpc/c/7a7d744ffe87ae10ab98004d1a6ca1f691af58e1

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/xive: use true and false for bool variable
From: Michael Ellerman @ 2021-03-31  1:09 UTC (permalink / raw)
  To: mpe, Yang Li; +Cc: paulus, linuxppc-dev, linux-kernel
In-Reply-To: <1615793096-83758-1-git-send-email-yang.lee@linux.alibaba.com>

On Mon, 15 Mar 2021 15:24:56 +0800, Yang Li wrote:
> fixed the following coccicheck:
> ./arch/powerpc/sysdev/xive/spapr.c:552:8-9: WARNING: return of 0/1 in
> function 'xive_spapr_match' with return type bool

Applied to powerpc/next.

[1/1] powerpc/xive: use true and false for bool variable
      https://git.kernel.org/powerpc/c/9214cf0f48cac3c6aa86f34e14969b5eccb72fad

cheers

^ permalink raw reply

* Re: [PATCH] cxl: Fix couple of spellings
From: Michael Ellerman @ 2021-03-31  1:09 UTC (permalink / raw)
  To: linuxppc-dev, gregkh, Bhaskar Chowdhury, arnd, fbarrat,
	linux-kernel, ajd
  Cc: rdunlap
In-Reply-To: <20210322023307.168754-1-unixbhaskar@gmail.com>

On Mon, 22 Mar 2021 08:03:07 +0530, Bhaskar Chowdhury wrote:
> s/filesytem/filesystem/
> s/symantics/semantics/

Applied to powerpc/next.

[1/1] cxl: Fix couple of spellings
      https://git.kernel.org/powerpc/c/e23ecdf9fd87c547a3ac55bcebaf7df28df2fab0

cheers

^ permalink raw reply

* Re: [PATCH v4 01/46] KVM: PPC: Book3S HV: Nested move LPCR sanitising to sanitise_hv_regs
From: Paul Mackerras @ 2021-03-31  2:47 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20210323010305.1045293-2-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:20AM +1000, Nicholas Piggin wrote:
> This will get a bit more complicated in future patches. Move it
> into the helper function.

This does change L1-visible behaviour, because now the L1 hypervisor
can see the LPCR bits that L0 is using, whereas previously it couldn't
(and that was deliberate).  I can't point to a specific scenario where
that is a real problem, but nevertheless it worries me.  And the
behaviour change should have been mentioned in the commit message at
least.

Paul.

^ permalink raw reply

* Re: [PATCH v4 02/46] KVM: PPC: Book3S HV: Add a function to filter guest LPCR bits
From: Paul Mackerras @ 2021-03-31  4:08 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20210323010305.1045293-3-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:21AM +1000, Nicholas Piggin wrote:
> Guest LPCR depends on hardware type, and future changes will add
> restrictions based on errata and guest MMU mode. Move this logic
> to a common function and use it for the cases where the guest
> wants to update its LPCR (or the LPCR of a nested guest).

[snip]

> @@ -4641,8 +4662,9 @@ void kvmppc_update_lpcr(struct kvm *kvm, unsigned long lpcr, unsigned long mask)
>  		struct kvmppc_vcore *vc = kvm->arch.vcores[i];
>  		if (!vc)
>  			continue;
> +
>  		spin_lock(&vc->lock);
> -		vc->lpcr = (vc->lpcr & ~mask) | lpcr;
> +		vc->lpcr = kvmppc_filter_lpcr_hv(vc, (vc->lpcr & ~mask) | lpcr);

This change seems unnecessary, since kvmppc_update_lpcr is called only
to update MMU configuration bits, not as a result of any action by
userspace or a nested hypervisor.  It's also beyond the scope of what
was mentioned in the commit message.

Paul.

^ permalink raw reply

* Re: [PATCH v4 03/46] KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2
From: Paul Mackerras @ 2021-03-31  4:28 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <20210323010305.1045293-4-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:22AM +1000, Nicholas Piggin wrote:
> These are already disallowed by H_SET_MODE from the guest, also disallow
> these by updating LPCR directly.
> 
> AIL modes can affect the host interrupt behaviour while the guest LPCR
> value is set, so filter it here too.
> 
> Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 04/46] KVM: PPC: Book3S HV: Prevent radix guests from setting LPCR[TC]
From: Paul Mackerras @ 2021-03-31  4:34 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20210323010305.1045293-5-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:23AM +1000, Nicholas Piggin wrote:
> This bit only applies to hash partitions.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kvm/book3s_hv.c        | 6 ++++++
>  arch/powerpc/kvm/book3s_hv_nested.c | 3 +--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index c5de7e3f22b6..1ffb0902e779 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1645,6 +1645,12 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu,
>   */
>  unsigned long kvmppc_filter_lpcr_hv(struct kvmppc_vcore *vc, unsigned long lpcr)
>  {
> +	struct kvm *kvm = vc->kvm;
> +
> +	/* LPCR_TC only applies to HPT guests */
> +	if (kvm_is_radix(kvm))
> +		lpcr &= ~LPCR_TC;

I'm not sure I see any benefit from this, and it is a little extra
complexity.

>  	/* On POWER8 and above, userspace can modify AIL */
>  	if (!cpu_has_feature(CPU_FTR_ARCH_207S))
>  		lpcr &= ~LPCR_AIL;
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index f7b441b3eb17..851e3f527eb2 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -140,8 +140,7 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
>  	/*
>  	 * Don't let L1 change LPCR bits for the L2 except these:
>  	 */
> -	mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
> -		LPCR_LPES | LPCR_MER;
> +	mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER;

Doesn't this make it completely impossible to set TC for any guest?

Paul.

^ permalink raw reply

* Re: [PATCH] powerpc/pseries: Only register vio drivers if vio bus exists
From: Michael Ellerman @ 2021-03-31  4:38 UTC (permalink / raw)
  To: Paul Menzel; +Cc: linuxppc-dev
In-Reply-To: <37664153-c31e-bd20-71b4-9afa049dc60a@molgen.mpg.de>

Paul Menzel <pmenzel@molgen.mpg.de> writes:
> Am 16.03.21 um 02:09 schrieb Michael Ellerman:
>> The vio bus is a fake bus, which we use on pseries LPARs (guests) to
>> discover devices provided by the hypervisor. There's no need or sense
>> in creating the vio bus on bare metal systems.
>> 
>> Which is why commit 4336b9337824 ("powerpc/pseries: Make vio and
>> ibmebus initcalls pseries specific") made the initialisation of the
>> vio bus only happen in LPARs.
>> 
>> However as a result of that commit we now see errors at boot on bare
>> metal systems:
>> 
>>    Driver 'hvc_console' was unable to register with bus_type 'vio' because the bus was not initialized.
>>    Driver 'tpm_ibmvtpm' was unable to register with bus_type 'vio' because the bus was not initialized.
>> 
>> This happens because those drivers are built-in, and are calling
>> vio_register_driver(). It in turn calls driver_register() with a
>> reference to vio_bus_type, but we haven't registered vio_bus_type with
>> the driver core.
>> 
>> Fix it by also guarding vio_register_driver() with a check to see if
>> we are on pseries.
>> 
>> Fixes: 4336b9337824 ("powerpc/pseries: Make vio and ibmebus initcalls pseries specific")
>> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>>   arch/powerpc/platforms/pseries/vio.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>> 
>> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
>> index 9cb4fc839fd5..429053d0402a 100644
>> --- a/arch/powerpc/platforms/pseries/vio.c
>> +++ b/arch/powerpc/platforms/pseries/vio.c
>> @@ -1285,6 +1285,10 @@ static int vio_bus_remove(struct device *dev)
>>   int __vio_register_driver(struct vio_driver *viodrv, struct module *owner,
>>   			  const char *mod_name)
>>   {
>> +	// vio_bus_type is only initialised for pseries
>> +	if (!machine_is(pseries))
>> +		return -ENODEV;
>> +
>>   	pr_debug("%s: driver %s registering\n", __func__, viodrv->name);
>>   
>>   	/* fill in 'struct driver' fields */
>
> Thank you. The errors are gone now.
>
> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> # IBM S822L (POWER8)
>
> As it fixes a commit from Linux 5.8, should it be tagged for the stable 
> releases, or is it going to be picked up automatically due to the Fixes tag?

It's not what I'd describe as a bad bug, so I'm not that inclined to
tag it for stable.

But given it has a Fixes tag, and is quite small, the bots will probably
backport it automatically.

cheers

^ permalink raw reply

* Re: [PATCH v4 06/46] KVM: PPC: Book3S HV: remove unused kvmppc_h_protect argument
From: Paul Mackerras @ 2021-03-31  4:38 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <20210323010305.1045293-7-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:25AM +1000, Nicholas Piggin wrote:
> The va argument is not used in the function or set by its asm caller,
> so remove it to be safe.
> 
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 05/46] KVM: PPC: Book3S HV: Remove redundant mtspr PSPB
From: Paul Mackerras @ 2021-03-31  4:35 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: Daniel Axtens, linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <20210323010305.1045293-6-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:24AM +1000, Nicholas Piggin wrote:
> This SPR is set to 0 twice when exiting the guest.
> 
> Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 07/46] KVM: PPC: Book3S HV: Fix CONFIG_SPAPR_TCE_IOMMU=n default hcalls
From: Paul Mackerras @ 2021-03-31  4:39 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <20210323010305.1045293-8-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:26AM +1000, Nicholas Piggin wrote:
> This config option causes the warning in init_default_hcalls to fire
> because the TCE handlers are in the default hcall list but not
> implemented.
> 
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 08/46] powerpc/64s: Remove KVM handler support from CBE_RAS interrupts
From: Paul Mackerras @ 2021-03-31  4:41 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <20210323010305.1045293-9-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:27AM +1000, Nicholas Piggin wrote:
> Cell does not support KVM.
> 
> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 09/46] powerpc/64s: remove KVM SKIP test from instruction breakpoint handler
From: Paul Mackerras @ 2021-03-31  4:54 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: Fabiano Rosas, linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <20210323010305.1045293-10-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:28AM +1000, Nicholas Piggin wrote:
> The code being executed in KVM_GUEST_MODE_SKIP is hypervisor code with
> MSR[IR]=0, so the faults of concern are the d-side ones caused by access
> to guest context by the hypervisor.
> 
> Instruction breakpoint interrupts are not a concern here. It's unlikely
> any good would come of causing breaks in this code, but skipping the
> instruction that caused it won't help matters (e.g., skip the mtmsr that
> sets MSR[DR]=0 or clears KVM_GUEST_MODE_SKIP).
> 
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

It might be worth noting in the commit message that the 0x1300
interrupt was dropped from the architecture a long time ago and is not
generated by P7, P8, P9 or P10.

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 10/46] KVM: PPC: Book3S HV: Ensure MSR[ME] is always set in guest MSR
From: Paul Mackerras @ 2021-03-31  4:56 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: Fabiano Rosas, linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <20210323010305.1045293-11-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:29AM +1000, Nicholas Piggin wrote:
> Rather than add the ME bit to the MSR at guest entry, make it clear
> that the hypervisor does not allow the guest to clear the bit.
> 
> The ME set is kept in guest entry for now, but a future patch will
> warn if it's not present.
> 
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 11/46] KVM: PPC: Book3S HV: Ensure MSR[HV] is always clear in guest MSR
From: Paul Mackerras @ 2021-03-31  4:58 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20210323010305.1045293-12-npiggin@gmail.com>

On Tue, Mar 23, 2021 at 11:02:30AM +1000, Nicholas Piggin wrote:
> Rather than clear the HV bit from the MSR at guest entry, make it clear
> that the hypervisor does not allow the guest to set the bit.
> 
> The HV clear is kept in guest entry for now, but a future patch will
> warn if it's not present.

Will warn if it *is* present, surely?

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH printk v2 2/5] printk: remove safe buffers
From: John Ogness @ 2021-03-31  7:59 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Alexey Kardashevskiy, Paul Mackerras,
	Tiezhu Yang, Rafael Aquini, Aneesh Kumar K.V, Peter Zijlstra,
	Yue Hu, Jordan Niethe, Kees Cook, Paul E. McKenney,
	Alistair Popple, Guilherme G. Piccoli, Nicholas Piggin,
	Steven Rostedt, Thomas Gleixner, kexec, linux-kernel,
	Sergey Senozhatsky, Eric Biederman, Andrew Morton, linuxppc-dev
In-Reply-To: <20210330153512.1182-3-john.ogness@linutronix.de>

On 2021-03-30, John Ogness <john.ogness@linutronix.de> wrote:
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index e971c0a9ec9e..f090d6a1b39e 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1772,16 +1759,21 @@ static struct task_struct *console_owner;
>  static bool console_waiter;
>  
>  /**
> - * console_lock_spinning_enable - mark beginning of code where another
> + * console_lock_spinning_enable_irqsave - mark beginning of code where another
>   *	thread might safely busy wait
>   *
>   * This basically converts console_lock into a spinlock. This marks
>   * the section where the console_lock owner can not sleep, because
>   * there may be a waiter spinning (like a spinlock). Also it must be
>   * ready to hand over the lock at the end of the section.
> + *
> + * This disables interrupts because the hand over to a waiter must not be
> + * interrupted until the hand over is completed (@console_waiter is cleared).
>   */
> -static void console_lock_spinning_enable(void)
> +static void console_lock_spinning_enable_irqsave(unsigned long *flags)

I missed the prototype change for the !CONFIG_PRINTK case, resulting in:

linux/kernel/printk/printk.c:2707:3: error: implicit declaration of function ‘console_lock_spinning_enable_irqsave’; did you mean ‘console_lock_spinning_enable’? [-Werror=implicit-function-declaration]
   console_lock_spinning_enable_irqsave(&flags);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   console_lock_spinning_enable

Will be fixed for v3.

(I have now officially added !CONFIG_PRINTK to my CI tests.)

John Ogness

^ permalink raw reply

* Re: [PATCH] PCI: Try to find two continuous regions for child resource
From: Kai-Heng Feng @ 2021-03-31  8:53 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Qian Cai, Michal Simek, Chen Zhou, Arnd Bergmann,
	open list:SPARC + UltraSPARC (sparc/sparc64),
	Alexey Kardashevskiy, open list:PCI SUBSYSTEM, open list,
	Dominik Brodowski, Frederic Barrat, Qinglang Miao, Paul Mackerras,
	Cédric Le Goater, Bjorn Helgaas, Oliver O'Halloran,
	Andrew Morton, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	David S. Miller, Mike Rapoport
In-Reply-To: <20210329162250.GA1167957@bjorn-Precision-5520>

On Tue, Mar 30, 2021 at 12:23 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Mar 29, 2021 at 04:47:59PM +0800, Kai-Heng Feng wrote:
> > Built-in grahpics on HP EliteDesk 805 G6 doesn't work because graphics
> > can't get the BAR it needs:
> > [    0.611504] pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window]
> > [    0.611505] pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window]
> > ...
> > [    0.638083] pci 0000:00:08.1:   bridge window [mem 0xd2000000-0xd23fffff]
> > [    0.638086] pci 0000:00:08.1:   bridge window [mem 0x10030000000-0x100401fffff 64bit pref]
> > [    0.962086] pci 0000:00:08.1: can't claim BAR 15 [mem 0x10030000000-0x100401fffff 64bit pref]: no compatible bridge window
> > [    0.962086] pci 0000:00:08.1: [mem 0x10030000000-0x100401fffff 64bit pref] clipped to [mem 0x10030000000-0x100303fffff 64bit pref]
> > [    0.962086] pci 0000:00:08.1:   bridge window [mem 0x10030000000-0x100303fffff 64bit pref]
> > [    0.962086] pci 0000:07:00.0: can't claim BAR 0 [mem 0x10030000000-0x1003fffffff 64bit pref]: no compatible bridge window
> > [    0.962086] pci 0000:07:00.0: can't claim BAR 2 [mem 0x10040000000-0x100401fffff 64bit pref]: no compatible bridge window
> >
> > However, the root bus has two continuous regions that can contain the
> > child resource requested.
> >
> > So try to find another parent region if two regions are continuous and
> > can contain child resource. This change makes the grahpics works on the
> > system in question.
>
> The BIOS description of PCI0 is interesting:
>
>   pci_bus 0000:00: root bus resource [mem 0x10000000000-0x100201fffff window]
>   pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window]
>   pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window]
>
> So the PCI0 _CRS apparently gave us:
>
>   [mem 0x10000000000-0x100201fffff] size 0x20200000 (512MB + 2MB)
>   [mem 0x10020200000-0x100303fffff] size 0x10200000 (256MB + 2MB)
>   [mem 0x10030400000-0x100401fffff] size 0x0fe00000 (254MB)
>
> These are all contiguous, so we'd have no problem if we coalesced them
> into a single window:
>
>   [mem 0x10000000000-0x100401fffff window] size 0x40200000 (1GB + 2MB)
>
> I think we currently keep these root bus resources separate because if
> we ever support _SRS for host bridges, the argument we give to _SRS
> must be exactly the same format as what we got from _CRS (see ACPI
> v6.3, sec 6.2.16, and pnpacpi_set_resources()).
>
> pnpacpi_encode_resources() is currently very simple-minded and copies
> each device resource back into a single _SRS entry.  But (1) we don't
> support _SRS for host bridges, and (2) if we ever do, we can make
> pnpacpi_encode_resources() smarter so it breaks things back up.
>
> So I think we should try to fix this by coalescing these adjacent
> resources from _CRS so we end up with a single root bus resource that
> covers all contiguous regions.

Thanks for the tip! Working on v2 patch.

>
> Typos, etc:
>   - No need for the timestamps; they're not relevant to the problem.
>   - s/grahpics/graphics/ (two occurrences above)
>   - s/continuous/contiguous/ (three occurrences above)

Will also update those in v2.

Kai-Heng

>
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212013
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> >  arch/microblaze/pci/pci-common.c |  4 +--
> >  arch/powerpc/kernel/pci-common.c |  8 ++---
> >  arch/sparc/kernel/pci.c          |  4 +--
> >  drivers/pci/pci.c                | 60 +++++++++++++++++++++++++++-----
> >  drivers/pci/setup-res.c          | 21 +++++++----
> >  drivers/pcmcia/rsrc_nonstatic.c  |  4 +--
> >  include/linux/pci.h              |  6 ++--
> >  7 files changed, 80 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> > index 557585f1be41..8e65832fb510 100644
> > --- a/arch/microblaze/pci/pci-common.c
> > +++ b/arch/microblaze/pci/pci-common.c
> > @@ -669,7 +669,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> >  {
> >       struct pci_bus *b;
> >       int i;
> > -     struct resource *res, *pr;
> > +     struct resource *res, *pr = NULL;
> >
> >       pr_debug("PCI: Allocating bus resources for %04x:%02x...\n",
> >                pci_domain_nr(bus), bus->number);
> > @@ -688,7 +688,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> >                        * and as such ensure proper re-allocation
> >                        * later.
> >                        */
> > -                     pr = pci_find_parent_resource(bus->self, res);
> > +                     pci_find_parent_resource(bus->self, res, &pr, NULL);
> >                       if (pr == res) {
> >                               /* this happens when the generic PCI
> >                                * code (wrongly) decides that this
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index 001e90cd8948..f865354b746d 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1196,7 +1196,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> >  {
> >       struct pci_bus *b;
> >       int i;
> > -     struct resource *res, *pr;
> > +     struct resource *res, *pr = NULL;
> >
> >       pr_debug("PCI: Allocating bus resources for %04x:%02x...\n",
> >                pci_domain_nr(bus), bus->number);
> > @@ -1213,7 +1213,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> >                       pr = (res->flags & IORESOURCE_IO) ?
> >                               &ioport_resource : &iomem_resource;
> >               else {
> > -                     pr = pci_find_parent_resource(bus->self, res);
> > +                     pci_find_parent_resource(bus->self, res, &pr, NULL);
> >                       if (pr == res) {
> >                               /* this happens when the generic PCI
> >                                * code (wrongly) decides that this
> > @@ -1265,12 +1265,12 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> >
> >  static inline void alloc_resource(struct pci_dev *dev, int idx)
> >  {
> > -     struct resource *pr, *r = &dev->resource[idx];
> > +     struct resource *pr = NULL, *r = &dev->resource[idx];
> >
> >       pr_debug("PCI: Allocating %s: Resource %d: %pR\n",
> >                pci_name(dev), idx, r);
> >
> > -     pr = pci_find_parent_resource(dev, r);
> > +     pci_find_parent_resource(dev, r, &pr, NULL);
> >       if (!pr || (pr->flags & IORESOURCE_UNSET) ||
> >           request_resource(pr, r) < 0) {
> >               printk(KERN_WARNING "PCI: Cannot allocate resource region %d"
> > diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
> > index 9c2b720bfd20..b4006798e4e1 100644
> > --- a/arch/sparc/kernel/pci.c
> > +++ b/arch/sparc/kernel/pci.c
> > @@ -621,7 +621,7 @@ static void pci_bus_register_of_sysfs(struct pci_bus *bus)
> >  static void pci_claim_legacy_resources(struct pci_dev *dev)
> >  {
> >       struct pci_bus_region region;
> > -     struct resource *p, *root, *conflict;
> > +     struct resource *p, *root = NULL, *conflict;
> >
> >       if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> >               return;
> > @@ -637,7 +637,7 @@ static void pci_claim_legacy_resources(struct pci_dev *dev)
> >       region.end = region.start + 0x1ffffUL;
> >       pcibios_bus_to_resource(dev->bus, p, &region);
> >
> > -     root = pci_find_parent_resource(dev, p);
> > +     pci_find_parent_resource(dev, p, &root, NULL);
> >       if (!root) {
> >               pci_info(dev, "can't claim VGA legacy %pR: no compatible bridge window\n", p);
> >               goto err;
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 16a17215f633..abbcd2dcdc02 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -693,20 +693,25 @@ u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
> >  EXPORT_SYMBOL_GPL(pci_find_ht_capability);
> >
> >  /**
> > - * pci_find_parent_resource - return resource region of parent bus of given
> > + * pci_find_parent_resource - find resource region of parent bus of given
> >   *                         region
> >   * @dev: PCI device structure contains resources to be searched
> >   * @res: child resource record for which parent is sought
> > + * @first: the first region that contains the child resource
> > + * @second: the second region that combines with the first region to fully
> > + * contains the child resource
> >   *
> >   * For given resource region of given device, return the resource region of
> >   * parent bus the given region is contained in.
> >   */
> > -struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > -                                       struct resource *res)
> > +void pci_find_parent_resource(const struct pci_dev *dev,
> > +                                       struct resource *res,
> > +                                       struct resource **first,
> > +                                       struct resource **second)
> >  {
> >       const struct pci_bus *bus = dev->bus;
> >       struct resource *r;
> > -     int i;
> > +     int i, overlaps = 0;
> >
> >       pci_bus_for_each_resource(bus, r, i) {
> >               if (!r)
> > @@ -718,8 +723,10 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> >                        * not, the allocator made a mistake.
> >                        */
> >                       if (r->flags & IORESOURCE_PREFETCH &&
> > -                         !(res->flags & IORESOURCE_PREFETCH))
> > -                             return NULL;
> > +                         !(res->flags & IORESOURCE_PREFETCH)) {
> > +                             *first = NULL;
> > +                             return;
> > +                     }
> >
> >                       /*
> >                        * If we're below a transparent bridge, there may
> > @@ -729,10 +736,47 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> >                        * on pci_bus_for_each_resource() giving us those
> >                        * first.
> >                        */
> > -                     return r;
> > +                     *first = r;
> > +                     return;
> >               }
> >       }
> > -     return NULL;
> > +
> > +     if (!second)
> > +             return;
> > +
> > +     pci_bus_for_each_resource(bus, r, i) {
> > +             if (!r)
> > +                     continue;
> > +             if (resource_overlaps(r, res)) {
> > +                     if (r->flags & IORESOURCE_PREFETCH &&
> > +                         !(res->flags & IORESOURCE_PREFETCH))
> > +                             continue;
> > +
> > +                     if (!overlaps++)
> > +                             *first = r;
> > +                     else {
> > +                             *second = r;
> > +                             break;
> > +                     }
> > +             }
> > +     }
> > +
> > +     if (overlaps != 2)
> > +             goto out;
> > +
> > +     if ((*first)->start > (*second)->start)
> > +             swap(*first, *second);
> > +
> > +     if ((*first)->end + 1 != (*second)->start)
> > +             goto out;
> > +
> > +     if ((*first)->start <= res->start && (*second)->end >= res->end)
> > +             return;
> > +out:
> > +
> > +     *first = NULL;
> > +     *second = NULL;
> > +     return;
> >  }
> >  EXPORT_SYMBOL(pci_find_parent_resource);
> >
> > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > index 7f1acb3918d0..e39615321d81 100644
> > --- a/drivers/pci/setup-res.c
> > +++ b/drivers/pci/setup-res.c
> > @@ -131,7 +131,7 @@ void pci_update_resource(struct pci_dev *dev, int resno)
> >  int pci_claim_resource(struct pci_dev *dev, int resource)
> >  {
> >       struct resource *res = &dev->resource[resource];
> > -     struct resource *root, *conflict;
> > +     struct resource *first = NULL, *second = NULL, *conflict;
> >
> >       if (res->flags & IORESOURCE_UNSET) {
> >               pci_info(dev, "can't claim BAR %d %pR: no address assigned\n",
> > @@ -147,21 +147,28 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
> >       if (res->flags & IORESOURCE_ROM_SHADOW)
> >               return 0;
> >
> > -     root = pci_find_parent_resource(dev, res);
> > -     if (!root) {
> > +     pci_find_parent_resource(dev, res, &first, &second);
> > +     if (!first) {
> >               pci_info(dev, "can't claim BAR %d %pR: no compatible bridge window\n",
> >                        resource, res);
> >               res->flags |= IORESOURCE_UNSET;
> >               return -EINVAL;
> >       }
> >
> > -     conflict = request_resource_conflict(root, res);
> > +     if (second)
> > +             first->end = second->end;
> > +
> > +     conflict = request_resource_conflict(first, res);
> >       if (conflict) {
> > +             if (second)
> > +                     first->end = second->start - 1;
> > +
> >               pci_info(dev, "can't claim BAR %d %pR: address conflict with %s %pR\n",
> >                        resource, res, conflict->name, conflict);
> >               res->flags |= IORESOURCE_UNSET;
> >               return -EBUSY;
> > -     }
> > +     } else if (second)
> > +             second->start = second->end = 0;
> >
> >       return 0;
> >  }
> > @@ -195,7 +202,7 @@ resource_size_t __weak pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx)
> >  static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
> >               int resno, resource_size_t size)
> >  {
> > -     struct resource *root, *conflict;
> > +     struct resource *root = NULL, *conflict;
> >       resource_size_t fw_addr, start, end;
> >
> >       fw_addr = pcibios_retrieve_fw_addr(dev, resno);
> > @@ -208,7 +215,7 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
> >       res->end = res->start + size - 1;
> >       res->flags &= ~IORESOURCE_UNSET;
> >
> > -     root = pci_find_parent_resource(dev, res);
> > +     pci_find_parent_resource(dev, res, &root, NULL);
> >       if (!root) {
> >               if (res->flags & IORESOURCE_IO)
> >                       root = &ioport_resource;
> > diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
> > index 3b05760e69d6..2fba42d7486e 100644
> > --- a/drivers/pcmcia/rsrc_nonstatic.c
> > +++ b/drivers/pcmcia/rsrc_nonstatic.c
> > @@ -73,7 +73,7 @@ static struct resource *
> >  claim_region(struct pcmcia_socket *s, resource_size_t base,
> >               resource_size_t size, int type, char *name)
> >  {
> > -     struct resource *res, *parent;
> > +     struct resource *res, *parent = NULL;
> >
> >       parent = type & IORESOURCE_MEM ? &iomem_resource : &ioport_resource;
> >       res = pcmcia_make_resource(base, size, type | IORESOURCE_BUSY, name);
> > @@ -81,7 +81,7 @@ claim_region(struct pcmcia_socket *s, resource_size_t base,
> >       if (res) {
> >  #ifdef CONFIG_PCI
> >               if (s && s->cb_dev)
> > -                     parent = pci_find_parent_resource(s->cb_dev, res);
> > +                     pci_find_parent_resource(s->cb_dev, res, &parent, NULL);
> >  #endif
> >               if (!parent || request_resource(parent, res)) {
> >                       kfree(res);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 86c799c97b77..dd1455be5247 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1049,8 +1049,10 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus);
> >  unsigned int pci_scan_child_bus(struct pci_bus *bus);
> >  void pci_bus_add_device(struct pci_dev *dev);
> >  void pci_read_bridge_bases(struct pci_bus *child);
> > -struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > -                                       struct resource *res);
> > +void pci_find_parent_resource(const struct pci_dev *dev,
> > +                                       struct resource *res,
> > +                                       struct resource **first,
> > +                                       struct resource **second);
> >  u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
> >  int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
> >  u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
> > --
> > 2.30.2
> >

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o
From: Christophe Leroy @ 2021-03-31  9:03 UTC (permalink / raw)
  To: Masahiro Yamada, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, linuxppc-dev
  Cc: Ravi Bangoria, Linux Kernel Mailing List, Nicholas Piggin,
	Oliver O'Halloran, Greentime Hu, Michal Suchanek,
	Ard Biesheuvel, Daniel Axtens
In-Reply-To: <CAK7LNASEVM8e5hohV4jbXOvMxSJ_Prm3es+fhezPkRc6UL=vdw@mail.gmail.com>



Le 28/01/2021 à 05:01, Masahiro Yamada a écrit :
> On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
>> enough to fix the issue. Kbuild is correctly rebuilding it because the
>> command line is changed.
>>
>> PowerPC builds each vdso directory twice; first in vdso_prepare to
>> generate vdso{32,64}-offsets.h, second as part of the ordinary build
>> process to embed vdso{32,64}.so.dbg into the kernel.
>>
>> The problem shows up when CONFIG_PPC_WERROR=y due to the following line
>> in arch/powerpc/Kbuild:
>>
>>    subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>>
>> In the preparation stage, Kbuild directly visits the vdso directories,
>> hence it does not inherit subdir-ccflags-y. In the second descend,
>> Kbuild adds -Werror, which results in the command line flipping
>> with/without -Werror.
>>
>> It implies a potential danger; if a more critical flag that would impact
>> the resulted vdso, the offsets recorded in the headers might be different
>> from real offsets in the embedded vdso images.
>>
>> Removing the unneeded second descend solves the problem.
>>
>> Link: https://lore.kernel.org/linuxppc-dev/87tuslxhry.fsf@mpe.ellerman.id.au/
>> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>> ---
> 
> 
> Michael, please take a  look at this.
> 
> The unneeded rebuild problem is still remaining.

Seems like with this patch, vdso32_wrapper.o is not rebuilt when vdso32.so.dbg is rebuilt.

Leading to ... disasters.

I'll send a patch


> 
> 
>>
>>   arch/powerpc/kernel/Makefile                      | 4 ++--
>>   arch/powerpc/kernel/vdso32/Makefile               | 5 +----
>>   arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S | 0
>>   arch/powerpc/kernel/vdso64/Makefile               | 6 +-----
>>   arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S | 0
>>   5 files changed, 4 insertions(+), 11 deletions(-)
>>   rename arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S (100%)
>>   rename arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S (100%)
>>
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index fe2ef598e2ea..79ee7750937d 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -51,7 +51,7 @@ obj-y                         += ptrace/
>>   obj-$(CONFIG_PPC64)            += setup_64.o \
>>                                     paca.o nvram_64.o note.o syscall_64.o
>>   obj-$(CONFIG_COMPAT)           += sys_ppc32.o signal_32.o
>> -obj-$(CONFIG_VDSO32)           += vdso32/
>> +obj-$(CONFIG_VDSO32)           += vdso32_wrapper.o
>>   obj-$(CONFIG_PPC_WATCHDOG)     += watchdog.o
>>   obj-$(CONFIG_HAVE_HW_BREAKPOINT)       += hw_breakpoint.o
>>   obj-$(CONFIG_PPC_DAWR)         += dawr.o
>> @@ -60,7 +60,7 @@ obj-$(CONFIG_PPC_BOOK3S_64)   += cpu_setup_power.o
>>   obj-$(CONFIG_PPC_BOOK3S_64)    += mce.o mce_power.o
>>   obj-$(CONFIG_PPC_BOOK3E_64)    += exceptions-64e.o idle_book3e.o
>>   obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
>> -obj-$(CONFIG_PPC64)            += vdso64/
>> +obj-$(CONFIG_PPC64)            += vdso64_wrapper.o
>>   obj-$(CONFIG_ALTIVEC)          += vecemu.o
>>   obj-$(CONFIG_PPC_BOOK3S_IDLE)  += idle_book3s.o
>>   procfs-y                       := proc_powerpc.o
>> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
>> index 59aa2944ecae..42fc3de89b39 100644
>> --- a/arch/powerpc/kernel/vdso32/Makefile
>> +++ b/arch/powerpc/kernel/vdso32/Makefile
>> @@ -30,7 +30,7 @@ CC32FLAGS += -m32
>>   KBUILD_CFLAGS := $(filter-out -mcmodel=medium,$(KBUILD_CFLAGS))
>>   endif
>>
>> -targets := $(obj-vdso32) vdso32.so.dbg
>> +targets := $(obj-vdso32) vdso32.so.dbg vgettimeofday.o
>>   obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
>>
>>   GCOV_PROFILE := n
>> @@ -46,9 +46,6 @@ obj-y += vdso32_wrapper.o
>>   targets += vdso32.lds
>>   CPPFLAGS_vdso32.lds += -P -C -Upowerpc
>>
>> -# Force dependency (incbin is bad)
>> -$(obj)/vdso32_wrapper.o : $(obj)/vdso32.so.dbg
>> -
>>   # link rule for the .so file, .lds has to be first
>>   $(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday.o FORCE
>>          $(call if_changed,vdso32ld_and_check)
>> diff --git a/arch/powerpc/kernel/vdso32/vdso32_wrapper.S b/arch/powerpc/kernel/vdso32_wrapper.S
>> similarity index 100%
>> rename from arch/powerpc/kernel/vdso32/vdso32_wrapper.S
>> rename to arch/powerpc/kernel/vdso32_wrapper.S
>> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
>> index d365810a689a..b50b39fedf74 100644
>> --- a/arch/powerpc/kernel/vdso64/Makefile
>> +++ b/arch/powerpc/kernel/vdso64/Makefile
>> @@ -17,7 +17,7 @@ endif
>>
>>   # Build rules
>>
>> -targets := $(obj-vdso64) vdso64.so.dbg
>> +targets := $(obj-vdso64) vdso64.so.dbg vgettimeofday.o
>>   obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
>>
>>   GCOV_PROFILE := n
>> @@ -29,15 +29,11 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
>>          -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
>>   asflags-y := -D__VDSO64__ -s
>>
>> -obj-y += vdso64_wrapper.o
>>   targets += vdso64.lds
>>   CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
>>
>>   $(obj)/vgettimeofday.o: %.o: %.c FORCE
>>
>> -# Force dependency (incbin is bad)
>> -$(obj)/vdso64_wrapper.o : $(obj)/vdso64.so.dbg
>> -
>>   # link rule for the .so file, .lds has to be first
>>   $(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
>>          $(call if_changed,vdso64ld_and_check)
>> diff --git a/arch/powerpc/kernel/vdso64/vdso64_wrapper.S b/arch/powerpc/kernel/vdso64_wrapper.S
>> similarity index 100%
>> rename from arch/powerpc/kernel/vdso64/vdso64_wrapper.S
>> rename to arch/powerpc/kernel/vdso64_wrapper.S
>> --
>> 2.27.0
>>
> 
> 

^ permalink raw reply

* [PATCH] powerpc/vdso: Make sure vdso_wrapper.o is rebuilt everytime vdso.so is rebuilt
From: Christophe Leroy @ 2021-03-31  9:12 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	masahiroy
  Cc: linuxppc-dev, linux-kernel

Commit bce74491c300 ("powerpc/vdso: fix unnecessary rebuilds of
vgettimeofday.o") moved vdso32_wrapper.o and vdso64_wrapper.o out
of arch/powerpc/kernel/vdso[32/64]/ and removed the dependencies in
the Makefile. This leads to the wrappers not being re-build hence the
kernel embedding the old vdso library.

Add back missing dependencies to ensure vdso32_wrapper.o and vdso64_wrapper.o
are rebuilt when vdso32.so.dbg and vdso64.so.dbg are changed.

Fixes: bce74491c300 ("powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o")
Cc: stable@vger.kernel.org
Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 6084fa499aa3..f66b63e81c3b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -191,3 +191,7 @@ $(obj)/prom_init_check: $(src)/prom_init_check.sh $(obj)/prom_init.o FORCE
 targets += prom_init_check
 
 clean-files := vmlinux.lds
+
+# Force dependency (incbin is bad)
+$(obj)/vdso32_wrapper.o : $(obj)/vdso32/vdso32.so.dbg
+$(obj)/vdso64_wrapper.o : $(obj)/vdso64/vdso64.so.dbg
-- 
2.25.0


^ permalink raw reply related

* Re: WARNING: CPU: 0 PID: 1 at arch/powerpc/lib/feature-fixups.c:109 do_feature_fixups+0xb0/0xf0
From: Christophe Leroy @ 2021-03-31  9:15 UTC (permalink / raw)
  To: Paul Menzel; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <bc1d3a33-2499-994e-860a-b50cf72b9619@molgen.mpg.de>

Hi Paul,

Le 30/03/2021 à 12:37, Paul Menzel a écrit :
> Dear Linux folks,
> 
> 
> On the POWER8 system IBM S822LC, Linux 5.12-rc5+ logs the warning below.
> 
> ```
> [    0.724118] Unable to patch feature section at (____ptrval____) - (____ptrval____) with 
> (____ptrval____) - (____ptrval____)
> [    0.724185] pstore: Registered nvram as persistent store backend
> ```
> 
> Please find the output of `dmesg` attached.
> 
> 

Did you do a 'make clean' before building ?

If not, can you try patch 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/8bb015bc98c51d8ced581415b7e3d157e18da7c9.1617181918.git.christophe.leroy@csgroup.eu/

Thanks
Christophe

^ permalink raw reply

* [PATCH -next] powerpc: Remove duplicated include from time.c
From: Qiheng Lin @ 2021-03-31  8:58 UTC (permalink / raw)
  To: linqiheng, Michael Ellerman, Nicholas Piggin, Christophe Leroy,
	Ingo Molnar, Geert Uytterhoeven, Peter Zijlstra,
	Frederic Weisbecker
  Cc: kernel-janitors, linuxppc-dev, linux-kernel, Hulk Robot

Remove duplicated include.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Qiheng Lin <linqiheng@huawei.com>
---
 arch/powerpc/kernel/time.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b67d93a609a2..2c8762002b21 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -53,7 +53,6 @@
 #include <linux/of_clk.h>
 #include <linux/suspend.h>
 #include <linux/sched/cputime.h>
-#include <linux/sched/clock.h>
 #include <linux/processor.h>
 #include <asm/trace.h>
 


^ permalink raw reply related

* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: Mike Rapoport @ 2021-03-31  9:28 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, linux-mips, linux-mm, Guo Ren, sparclinux,
	linux-riscv, Jonas Bonn, linux-s390, Yoshinori Sato,
	linux-hexagon, Huacai Chen, Russell King, linux-csky, Ingo Molnar,
	linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
	linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
	linux-parisc, linux-kernel, Palmer Dabbelt, linux-alpha,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>

On Wed, Mar 17, 2021 at 09:52:10AM +0800, Kefeng Wang wrote:
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Acked-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
> v2:
> - Cleanup 'str' line suggested by Christophe and ACK
> 

^ permalink raw reply

* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: Russell King - ARM Linux admin @ 2021-03-31  9:34 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, linux-mips, linux-mm, Guo Ren, sparclinux,
	linux-riscv, Jonas Bonn, linux-s390, Yoshinori Sato,
	linux-hexagon, Huacai Chen, linux-csky, Ingo Molnar,
	linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
	linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
	linux-parisc, linux-kernel, Palmer Dabbelt, linux-alpha,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>

On Wed, Mar 17, 2021 at 09:52:10AM +0800, Kefeng Wang wrote:
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Acked-by: Russell King <rmk+kernel@armlinux.org.uk> # for arm bits
-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Michael Ellerman @ 2021-03-31  9:58 UTC (permalink / raw)
  To: Xiongwei Song, benh, paulus, oleg, npiggin, christophe.leroy,
	msuchanek, aneesh.kumar, ravi.bangoria, mikey, haren, alistair,
	jniethe5, peterz, leobras.c, akpm, rppt, peterx, atrajeev, maddy,
	kjain, kan.liang, aik, pmladek, john.ogness
  Cc: Xiongwei Song, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20210330150425.10145-1-sxwjean@me.com>

Xiongwei Song <sxwjean@me.com> writes:
> From: Xiongwei Song <sxwjean@gmail.com>
>
> Create a new header named traps.h, define macros to list ppc exception
> types in traps.h, replace the reference of the real trap values with
> these macros.

Personally I find the hex values easier to recognise, but I realise
that's probably not true of other people :)

...
> diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h
> new file mode 100644
> index 000000000000..a31b6122de23
> --- /dev/null
> +++ b/arch/powerpc/include/asm/traps.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_PPC_TRAPS_H
> +#define _ASM_PPC_TRAPS_H
> +
> +#define TRAP_RESET   0x100 /* System reset */
> +#define TRAP_MCE     0x200 /* Machine check */
> +#define TRAP_DSI     0x300 /* Data storage */
> +#define TRAP_DSEGI   0x380 /* Data segment */
> +#define TRAP_ISI     0x400 /* Instruction storage */
> +#define TRAP_ISEGI   0x480 /* Instruction segment */
> +#define TRAP_ALIGN   0x600 /* Alignment */
> +#define TRAP_PROG    0x700 /* Program */
> +#define TRAP_DEC     0x900 /* Decrementer */
> +#define TRAP_SYSCALL 0xc00 /* System call */
> +#define TRAP_TRACEI  0xd00 /* Trace */
> +#define TRAP_FPA     0xe00 /* Floating-point Assist */
> +#define TRAP_PMI     0xf00 /* Performance monitor */

I know the macro is called TRAP and the field in pt_regs is called trap,
but the terminology in the architecture is "exception", and we already
have many uses of that. In particular we have a lot of uses of "exc" as
an abbreviation for "exception". So I think I'd rather we use that than
"TRAP".

I think we should probably use the names from the ISA, unless they are
really over long.

Which are:

  0x100   System Reset
  0x200   Machine Check
  0x300   Data Storage
  0x380   Data Segment
  0x400   Instruction Storage
  0x480   Instruction Segment
  0x500   External
  0x600   Alignment
  0x700   Program
  0x800   Floating-Point Unavailable
  0x900   Decrementer
  0x980   Hypervisor Decrementer
  0xA00   Directed Privileged Doorbell
  0xC00   System Call
  0xD00   Trace
  0xE00   Hypervisor Data Storage
  0xE20   Hypervisor Instruction Storage
  0xE40   Hypervisor Emulation Assistance
  0xE60   Hypervisor Maintenance
  0xE80   Directed Hypervisor Doorbell
  0xEA0   Hypervisor Virtualization
  0xF00   Performance Monitor
  0xF20   Vector Unavailable
  0xF40   VSX Unavailable
  0xF60   Facility Unavailable
  0xF80   Hypervisor Facility Unavailable
  0xFA0   Directed Ultravisor Doorbell


So perhaps:

  EXC_SYSTEM_RESET
  EXC_MACHINE_CHECK
  EXC_DATA_STORAGE
  EXC_DATA_SEGMENT
  EXC_INST_STORAGE
  EXC_INST_SEGMENT
  EXC_EXTERNAL_INTERRUPT
  EXC_ALIGNMENT
  EXC_PROGRAM_CHECK
  EXC_FP_UNAVAILABLE
  EXC_DECREMENTER
  EXC_HV_DECREMENTER
  EXC_SYSTEM_CALL
  EXC_HV_DATA_STORAGE
  EXC_PERF_MONITOR


cheers

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox