* [powerpc:next 231/236] arch/powerpc/kernel/fadump.c:731:28: error: 'INTERRUPT_SYSTEM_RESET' undeclared
From: kernel test robot @ 2021-04-19 22:27 UTC (permalink / raw)
To: Xiongwei Song; +Cc: linuxppc-dev, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3966 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
head: cbd3d5ba46b68c033986a6087209930f001cbcca
commit: 7153d4bf0b373428d0393c001019da4d0483fddb [231/236] powerpc/traps: Enhance readability for trap types
config: powerpc-allyesconfig (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://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=7153d4bf0b373428d0393c001019da4d0483fddb
git remote add powerpc https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
git fetch --no-tags powerpc next
git checkout 7153d4bf0b373428d0393c001019da4d0483fddb
# 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>
Note: the powerpc/next HEAD cbd3d5ba46b68c033986a6087209930f001cbcca builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
arch/powerpc/kernel/fadump.c:82:12: warning: no previous prototype for 'fadump_cma_init' [-Wmissing-prototypes]
82 | int __init fadump_cma_init(void)
| ^~~~~~~~~~~~~~~
arch/powerpc/kernel/fadump.c: In function 'crash_fadump':
>> arch/powerpc/kernel/fadump.c:731:28: error: 'INTERRUPT_SYSTEM_RESET' undeclared (first use in this function)
731 | if (TRAP(&(fdh->regs)) == INTERRUPT_SYSTEM_RESET) {
| ^~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/fadump.c:731:28: note: each undeclared identifier is reported only once for each function it appears in
vim +/INTERRUPT_SYSTEM_RESET +731 arch/powerpc/kernel/fadump.c
679
680 void crash_fadump(struct pt_regs *regs, const char *str)
681 {
682 unsigned int msecs;
683 struct fadump_crash_info_header *fdh = NULL;
684 int old_cpu, this_cpu;
685 /* Do not include first CPU */
686 unsigned int ncpus = num_online_cpus() - 1;
687
688 if (!should_fadump_crash())
689 return;
690
691 /*
692 * old_cpu == -1 means this is the first CPU which has come here,
693 * go ahead and trigger fadump.
694 *
695 * old_cpu != -1 means some other CPU has already on it's way
696 * to trigger fadump, just keep looping here.
697 */
698 this_cpu = smp_processor_id();
699 old_cpu = cmpxchg(&crashing_cpu, -1, this_cpu);
700
701 if (old_cpu != -1) {
702 atomic_inc(&cpus_in_fadump);
703
704 /*
705 * We can't loop here indefinitely. Wait as long as fadump
706 * is in force. If we race with fadump un-registration this
707 * loop will break and then we go down to normal panic path
708 * and reboot. If fadump is in force the first crashing
709 * cpu will definitely trigger fadump.
710 */
711 while (fw_dump.dump_registered)
712 cpu_relax();
713 return;
714 }
715
716 fdh = __va(fw_dump.fadumphdr_addr);
717 fdh->crashing_cpu = crashing_cpu;
718 crash_save_vmcoreinfo();
719
720 if (regs)
721 fdh->regs = *regs;
722 else
723 ppc_save_regs(&fdh->regs);
724
725 fdh->online_mask = *cpu_online_mask;
726
727 /*
728 * If we came in via system reset, wait a while for the secondary
729 * CPUs to enter.
730 */
> 731 if (TRAP(&(fdh->regs)) == INTERRUPT_SYSTEM_RESET) {
732 msecs = CRASH_TIMEOUT;
733 while ((atomic_read(&cpus_in_fadump) < ncpus) && (--msecs > 0))
734 mdelay(1);
735 }
736
737 fw_dump.ops->fadump_trigger(fdh, str);
738 }
739
---
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: 72728 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()
From: Michael Ellerman @ 2021-04-19 23:30 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, Daniel Axtens, robh, dan.carpenter
Cc: devicetree, linuxppc-dev, kbuild-all, bauerman, lkp
In-Reply-To: <2817d674-d420-580f-a0c1-b842da915a80@linux.microsoft.com>
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>
>> Daniel Axtens <dja@axtens.net> writes:
>>>> On 4/15/21 12:14 PM, Lakshmi Ramasubramanian wrote:
>>>>
>>>> Sorry - missed copying device-tree and powerpc mailing lists.
>>>>
>>>>> There are a few "goto out;" statements before the local variable "fdt"
>>>>> is initialized through the call to of_kexec_alloc_and_setup_fdt() in
>>>>> elf64_load(). This will result in an uninitialized "fdt" being passed
>>>>> to kvfree() in this function if there is an error before the call to
>>>>> of_kexec_alloc_and_setup_fdt().
>>>>>
>>>>> Initialize the local variable "fdt" to NULL.
>>>>>
>>> I'm a huge fan of initialising local variables! But I'm struggling to
>>> find the code path that will lead to an uninit fdt being returned...
>>>
>>> The out label reads in part:
>>>
>>> /* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>>> return ret ? ERR_PTR(ret) : fdt;
>>>
>>> As far as I can tell, any time we get a non-zero ret, we're going to
>>> return an error pointer rather than the uninitialised value...
>
> As Dan pointed out, the new code is in linux-next.
>
> I have copied the new one below - the function doesn't return fdt, but
> instead sets it in the arch specific field (please see the link to the
> updated elf_64.c below).
>
> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>
>>>
>>> (btw, it does look like we might leak fdt if we have an error after we
>>> successfully kmalloc it.)
>>>
>>> Am I missing something? Can you link to the report for the kernel test
>>> robot or from Dan?
>
> /*
> * Once FDT buffer has been successfully passed to
> kexec_add_buffer(),
> * the FDT buffer address is saved in image->arch.fdt. In that
> case,
> * the memory cannot be freed here in case of any other error.
> */
> if (ret && !image->arch.fdt)
> kvfree(fdt);
>
> return ret ? ERR_PTR(ret) : NULL;
>
> In case of an error, the memory allocated for fdt is freed unless it has
> already been passed to kexec_add_buffer().
It feels like the root of the problem is that the kvfree of fdt is in
the wrong place. It's only allocated later in the function, so the error
path should reflect that. Something like the patch below.
cheers
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 5a569bb51349..02662e72c53d 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
initrd_len, cmdline);
if (ret)
- goto out;
+ goto out_free_fdt;
fdt_pack(fdt);
@@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
ret = kexec_add_buffer(&kbuf);
if (ret)
- goto out;
+ goto out_free_fdt;
/* FDT will be freed in arch_kimage_file_post_load_cleanup */
image->arch.fdt = fdt;
@@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
if (ret)
pr_err("Error setting up the purgatory.\n");
+ goto out;
+
+out_free_fdt:
+ kvfree(fdt);
out:
kfree(modified_cmdline);
kexec_free_elf_info(&elf_info);
- /*
- * Once FDT buffer has been successfully passed to kexec_add_buffer(),
- * the FDT buffer address is saved in image->arch.fdt. In that case,
- * the memory cannot be freed here in case of any other error.
- */
- if (ret && !image->arch.fdt)
- kvfree(fdt);
-
return ret ? ERR_PTR(ret) : NULL;
}
^ permalink raw reply related
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Leonardo Bras @ 2021-04-20 0:35 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <CAL_Jsq+m6CkGj_NYGvwxoKwoQ4PkEu6hfGdMTT3i4APoHSkNeg@mail.gmail.com>
On Mon, 2021-04-19 at 10:44 -0500, Rob Herring wrote:
> On Fri, Apr 16, 2021 at 3:58 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> >
> > Hello Rob, thanks for this feedback!
> >
> > On Thu, 2021-04-15 at 13:59 -0500, Rob Herring wrote:
> > > +PPC and PCI lists
> > >
> > > On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > > >
> > > > Many other resource flag parsers already add this flag when the input
> > > > has bits 24 & 25 set, so update this one to do the same.
> > >
> > > Many others? Looks like sparc and powerpc to me.
> > >
> >
> > s390 also does that, but it look like it comes from a device-tree.
>
> I'm only looking at DT based platforms, and s390 doesn't use DT.
Correct.
Sorry, I somehow write above the opposite of what I was thinking.
>
> > > Those would be the
> > > ones I worry about breaking. Sparc doesn't use of/address.c so it's
> > > fine. Powerpc version of the flags code was only fixed in 2019, so I
> > > don't think powerpc will care either.
> >
> > In powerpc I reach this function with this stack, while configuring a
> > virtio-net device for a qemu/KVM pseries guest:
> >
> > pci_process_bridge_OF_ranges+0xac/0x2d4
> > pSeries_discover_phbs+0xc4/0x158
> > discover_phbs+0x40/0x60
> > do_one_initcall+0x60/0x2d0
> > kernel_init_freeable+0x308/0x3a8
> > kernel_init+0x2c/0x168
> > ret_from_kernel_thread+0x5c/0x70
> >
> > For this, both MMIO32 and MMIO64 resources will have flags 0x200.
>
> Oh good, powerpc has 2 possible flags parsing functions. So in the
> above path, do we need to set PCI_BASE_ADDRESS_MEM_TYPE_64?
>
> Does pci_parse_of_flags() get called in your case?
>
It's called in some cases, but not for the device I am debugging
(virtio-net pci@800000020000000).
For the above device, here is an expanded stack trace:
of_bus_pci_get_flags() (from parser->bus->get_flags())
of_pci_range_parser_one() (from macro for_each_of_pci_range)
pci_process_bridge_OF_ranges+0xac/0x2d4
pSeries_discover_phbs+0xc4/0x158
discover_phbs+0x40/0x60
do_one_initcall+0x60/0x2d0
kernel_init_freeable+0x308/0x3a8
kernel_init+0x2c/0x168
ret_from_kernel_thread+0x5c/0x70
For other devices, I could also see the following stack trace:
## device ethernet@8
pci_parse_of_flags()
of_create_pci_dev+0x7f0/0xa40
__of_scan_bus+0x248/0x320
pcibios_scan_phb+0x370/0x3b0
pcibios_init+0x8c/0x12c
do_one_initcall+0x60/0x2d0
kernel_init_freeable+0x308/0x3a8
kernel_init+0x2c/0x168
ret_from_kernel_thread+0x5c/0x70
Devices that get parsed with of_bus_pci_get_flags() appears first at
dmesg (around 0.015s in my test), while devices that get parsed by
pci_parse_of_flags() appears later (0.025s in my test).
I am not really used to this code, but having the term "discover phbs"
in the first trace and the term "scan phb" in the second, makes me
wonder if the first trace is seen on devices that are seen/described in
the device-tree and the second trace is seen in devices not present in
the device-tree and found scanning pci bus.
> > > I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
> > > the flags. AFAICT, that's not set anywhere outside of arch code. So
> > > never for riscv, arm and arm64 at least. That leads me to
> > > pci_std_update_resource() which is where the PCI code sets BARs and
> > > just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
> > > IORESOURCE_* flags. So it seems like 64-bit is still not handled and
> > > neither is prefetch.
> > >
> >
> > I am not sure if you mean here:
> > a) it's ok to add IORESOURCE_MEM_64 here, because it does not affect
> > anything else, or
> > b) it should be using PCI_BASE_ADDRESS_MEM_TYPE_64
> > (or IORESOURCE_MEM_64 | PCI_BASE_ADDRESS_MEM_TYPE_64) instead, since
> > it's how it's added in powerpc/sparc, and else there is no point.
>
> I'm wondering if a) is incomplete and PCI_BASE_ADDRESS_MEM_TYPE_64
> also needs to be set. The question is ultimately are BARs getting set
> correctly for 64-bit? It looks to me like they aren't.
I am not used to these terms, does BAR means 'Base Address Register'?
If so, those are the addresses stored in pci->phb->mem_resources[i] and
pci->phb->mem_offset[i], printed from enable_ddw() (which takes place a
lot after discovering the device (0.17s in my run)).
resource #1 pci@800000020000000: start=0x200080000000
end=0x2000ffffffff flags=0x200 desc=0x0 offset=0x200000000000
resource #2 pci@800000020000000: start=0x210000000000
end=0x21ffffffffff flags=0x200 desc=0x0 offset=0x0
The message above was printed without this patch.
With the patch, the flags for memory resource #2 gets ORed with
0x00100000.
Is it enough to know if BARs are correctly set for 64-bit?
If it's not, how can I check?
>
> Rob
Thanks Rob!
Leonardo Brás
^ permalink raw reply
* Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match
From: Dominique MARTINET @ 2021-04-19 23:42 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ulf Hansson, aymen.sghaier, Geert Uytterhoeven, Rafael Wysocki,
David Airlie, Michael Turquette, dri-devel,
Linux Kernel Mailing List, Andrzej Hajda, Networking, linux-phy,
peter.ujfalusi, linux-clk, Linux-Renesas, Wim Van Sebroeck,
Herbert Xu, Horia Geantă, Kevin Hilman, Joerg Roedel,
Neil Armstrong, linux-staging, open list:IOMMU DRIVERS, Kishon,
Tony Lindgren, linux-omap, Geert Uytterhoeven, Jakub Kicinski,
Linus Walleij, Guenter Roeck, Linux Media Mailing List,
LINUXWATCHDOG, Will Deacon, Linux PM list, linuxppc-dev,
Eduardo Valentin, open list:GPIO SUBSYSTEM,
moderated list:ARM/Mediatek SoC..., Santosh Shilimkar,
Matthias Brugger, open list:ARM/Amlogic Meson SoC support,
Mauro Carvalho Chehab, Linux ARM, Alice Guo (OSS), Felipe Balbi,
tomba, Stephen Boyd, gregkh, Alan Stern, USB list, linux-mmc,
Adrian Hunter, Robert Foss, Leo Li, Tony Prisk, Vinod Koul,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Daniel Vetter,
Keerthy, dmaengine, Roy Pledge, jyri.sarha, David Miller
In-Reply-To: <CAK8P3a1Mu2F0irDDCL-50HiHth29iYFL5b7WHZ=UX6W7zzoxAg@mail.gmail.com>
Arnd Bergmann wrote on Mon, Apr 19, 2021 at 02:16:36PM +0200:
> In some cases, you can use the device_link infrastructure to deal
> with dependencies between devices. Not sure if this would help
> in your case, but have a look at device_link_add() etc in drivers/base/core.c
I'll need to actually try to convince myself but if creating the link
forces driver registration then it should be workable.
> > In this particular case the problem is that since 7d981405d0fd ("soc:
> > imx8m: change to use platform driver") the soc probe tries to use the
> > nvmem driver for ocotp fuses for imx8m devices, which isn't ready yet.
> > So soc loading gets pushed back to the end of the list because it gets
> > defered and other drivers relying on soc_device_match get confused
> > because they wrongly think a device doesn't match a quirk when it
> > actually does.
> >
> > If there is a way to ensure the nvmem driver gets loaded before the soc,
> > that would also solve the problem nicely, and avoid the need to mess
> > with all the ~50 drivers which use it.
> >
> > Is there a way to control in what order drivers get loaded? Something in
> > the dtb perhaps?
>
> For built-in drivers, load order depends on the initcall level and
> link order (how things are lined listed in the Makefile hierarchy).
>
> For loadable modules, this is up to user space in the end.
>
> Which of the drivers in this scenario are loadable modules?
All the drivers involved in my case are built-in (nvmem, soc and final
soc_device_match consumer e.g. caam_jr that crashes the kernel if soc is
not identified properly).
I frankly don't like the idea of moving nvmem/ above soc/ in
drivers/Makefile as a "solution" to this (especially as there is one
that seems to care about what soc they run on...), so I'll have a look
at links first, hopefully that will work out.
Thanks,
--
Dominique
^ permalink raw reply
* Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()
From: Lakshmi Ramasubramanian @ 2021-04-20 1:33 UTC (permalink / raw)
To: Michael Ellerman, Daniel Axtens, robh, dan.carpenter
Cc: devicetree, linuxppc-dev, kbuild-all, bauerman, lkp
In-Reply-To: <87pmypdf93.fsf@mpe.ellerman.id.au>
On 4/19/21 4:30 PM, Michael Ellerman wrote:
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>>
>>> Daniel Axtens <dja@axtens.net> writes:
>>>>> On 4/15/21 12:14 PM, Lakshmi Ramasubramanian wrote:
>>>>>
>>>>> Sorry - missed copying device-tree and powerpc mailing lists.
>>>>>
>>>>>> There are a few "goto out;" statements before the local variable "fdt"
>>>>>> is initialized through the call to of_kexec_alloc_and_setup_fdt() in
>>>>>> elf64_load(). This will result in an uninitialized "fdt" being passed
>>>>>> to kvfree() in this function if there is an error before the call to
>>>>>> of_kexec_alloc_and_setup_fdt().
>>>>>>
>>>>>> Initialize the local variable "fdt" to NULL.
>>>>>>
>>>> I'm a huge fan of initialising local variables! But I'm struggling to
>>>> find the code path that will lead to an uninit fdt being returned...
>>>>
>>>> The out label reads in part:
>>>>
>>>> /* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>>>> return ret ? ERR_PTR(ret) : fdt;
>>>>
>>>> As far as I can tell, any time we get a non-zero ret, we're going to
>>>> return an error pointer rather than the uninitialised value...
>>
>> As Dan pointed out, the new code is in linux-next.
>>
>> I have copied the new one below - the function doesn't return fdt, but
>> instead sets it in the arch specific field (please see the link to the
>> updated elf_64.c below).
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>>
>>>>
>>>> (btw, it does look like we might leak fdt if we have an error after we
>>>> successfully kmalloc it.)
>>>>
>>>> Am I missing something? Can you link to the report for the kernel test
>>>> robot or from Dan?
>>
>> /*
>> * Once FDT buffer has been successfully passed to
>> kexec_add_buffer(),
>> * the FDT buffer address is saved in image->arch.fdt. In that
>> case,
>> * the memory cannot be freed here in case of any other error.
>> */
>> if (ret && !image->arch.fdt)
>> kvfree(fdt);
>>
>> return ret ? ERR_PTR(ret) : NULL;
>>
>> In case of an error, the memory allocated for fdt is freed unless it has
>> already been passed to kexec_add_buffer().
>
> It feels like the root of the problem is that the kvfree of fdt is in
> the wrong place. It's only allocated later in the function, so the error
> path should reflect that. Something like the patch below.
>
> cheers
>
>
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..02662e72c53d 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
> initrd_len, cmdline);
> if (ret)
> - goto out;
> + goto out_free_fdt;
>
> fdt_pack(fdt);
>
> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> - goto out;
> + goto out_free_fdt;
>
> /* FDT will be freed in arch_kimage_file_post_load_cleanup */
> image->arch.fdt = fdt;
> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> if (ret)
> pr_err("Error setting up the purgatory.\n");
>
> + goto out;
> +
> +out_free_fdt:
> + kvfree(fdt);
> out:
> kfree(modified_cmdline);
> kexec_free_elf_info(&elf_info);
>
> - /*
> - * Once FDT buffer has been successfully passed to kexec_add_buffer(),
> - * the FDT buffer address is saved in image->arch.fdt. In that case,
> - * the memory cannot be freed here in case of any other error.
> - */
> - if (ret && !image->arch.fdt)
> - kvfree(fdt);
> -
> return ret ? ERR_PTR(ret) : NULL;
> }
>
>
This looks good to me. Thanks Michael.
I'll post the updated patch shortly.
-lakshmi
^ permalink raw reply
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Rob Herring @ 2021-04-20 1:39 UTC (permalink / raw)
To: Leonardo Bras
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <b875ef1778e17a87ee1f4b71d26f2782831b1d07.camel@gmail.com>
On Mon, Apr 19, 2021 at 7:35 PM Leonardo Bras <leobras.c@gmail.com> wrote:
>
> On Mon, 2021-04-19 at 10:44 -0500, Rob Herring wrote:
> > On Fri, Apr 16, 2021 at 3:58 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > >
> > > Hello Rob, thanks for this feedback!
> > >
> > > On Thu, 2021-04-15 at 13:59 -0500, Rob Herring wrote:
> > > > +PPC and PCI lists
> > > >
> > > > On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > > > >
> > > > > Many other resource flag parsers already add this flag when the input
> > > > > has bits 24 & 25 set, so update this one to do the same.
> > > >
> > > > Many others? Looks like sparc and powerpc to me.
> > > >
> > >
> > > s390 also does that, but it look like it comes from a device-tree.
> >
> > I'm only looking at DT based platforms, and s390 doesn't use DT.
>
> Correct.
> Sorry, I somehow write above the opposite of what I was thinking.
>
> >
> > > > Those would be the
> > > > ones I worry about breaking. Sparc doesn't use of/address.c so it's
> > > > fine. Powerpc version of the flags code was only fixed in 2019, so I
> > > > don't think powerpc will care either.
> > >
> > > In powerpc I reach this function with this stack, while configuring a
> > > virtio-net device for a qemu/KVM pseries guest:
> > >
> > > pci_process_bridge_OF_ranges+0xac/0x2d4
> > > pSeries_discover_phbs+0xc4/0x158
> > > discover_phbs+0x40/0x60
> > > do_one_initcall+0x60/0x2d0
> > > kernel_init_freeable+0x308/0x3a8
> > > kernel_init+0x2c/0x168
> > > ret_from_kernel_thread+0x5c/0x70
> > >
> > > For this, both MMIO32 and MMIO64 resources will have flags 0x200.
> >
> > Oh good, powerpc has 2 possible flags parsing functions. So in the
> > above path, do we need to set PCI_BASE_ADDRESS_MEM_TYPE_64?
> >
> > Does pci_parse_of_flags() get called in your case?
> >
>
> It's called in some cases, but not for the device I am debugging
> (virtio-net pci@800000020000000).
>
> For the above device, here is an expanded stack trace:
>
> of_bus_pci_get_flags() (from parser->bus->get_flags())
> of_pci_range_parser_one() (from macro for_each_of_pci_range)
> pci_process_bridge_OF_ranges+0xac/0x2d4
> pSeries_discover_phbs+0xc4/0x158
> discover_phbs+0x40/0x60
> do_one_initcall+0x60/0x2d0
> kernel_init_freeable+0x308/0x3a8
> kernel_init+0x2c/0x168
> ret_from_kernel_thread+0x5c/0x70
>
> For other devices, I could also see the following stack trace:
> ## device ethernet@8
>
> pci_parse_of_flags()
> of_create_pci_dev+0x7f0/0xa40
> __of_scan_bus+0x248/0x320
> pcibios_scan_phb+0x370/0x3b0
> pcibios_init+0x8c/0x12c
> do_one_initcall+0x60/0x2d0
> kernel_init_freeable+0x308/0x3a8
> kernel_init+0x2c/0x168
> ret_from_kernel_thread+0x5c/0x70
>
> Devices that get parsed with of_bus_pci_get_flags() appears first at
> dmesg (around 0.015s in my test), while devices that get parsed by
> pci_parse_of_flags() appears later (0.025s in my test).
>
> I am not really used to this code, but having the term "discover phbs"
> in the first trace and the term "scan phb" in the second, makes me
> wonder if the first trace is seen on devices that are seen/described in
> the device-tree and the second trace is seen in devices not present in
> the device-tree and found scanning pci bus.
That was my guess as well. I think on pSeries that most PCI devices
are in the DT whereas on Arm and other flattened DT (non OpenFirmware)
platforms PCI devices are not in DT. Of course, for virtio devices,
they would not be in DT in either case.
> > > > I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
> > > > the flags. AFAICT, that's not set anywhere outside of arch code. So
> > > > never for riscv, arm and arm64 at least. That leads me to
> > > > pci_std_update_resource() which is where the PCI code sets BARs and
> > > > just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
> > > > IORESOURCE_* flags. So it seems like 64-bit is still not handled and
> > > > neither is prefetch.
> > > >
> > >
> > > I am not sure if you mean here:
> > > a) it's ok to add IORESOURCE_MEM_64 here, because it does not affect
> > > anything else, or
> > > b) it should be using PCI_BASE_ADDRESS_MEM_TYPE_64
> > > (or IORESOURCE_MEM_64 | PCI_BASE_ADDRESS_MEM_TYPE_64) instead, since
> > > it's how it's added in powerpc/sparc, and else there is no point.
> >
> > I'm wondering if a) is incomplete and PCI_BASE_ADDRESS_MEM_TYPE_64
> > also needs to be set. The question is ultimately are BARs getting set
> > correctly for 64-bit? It looks to me like they aren't.
>
> I am not used to these terms, does BAR means 'Base Address Register'?
Yes. Standard PCI thing.
> If so, those are the addresses stored in pci->phb->mem_resources[i] and
> pci->phb->mem_offset[i], printed from enable_ddw() (which takes place a
> lot after discovering the device (0.17s in my run)).
>
> resource #1 pci@800000020000000: start=0x200080000000
> end=0x2000ffffffff flags=0x200 desc=0x0 offset=0x200000000000
> resource #2 pci@800000020000000: start=0x210000000000
> end=0x21ffffffffff flags=0x200 desc=0x0 offset=0x0
>
> The message above was printed without this patch.
> With the patch, the flags for memory resource #2 gets ORed with
> 0x00100000.
Right, as expected.
> Is it enough to know if BARs are correctly set for 64-bit?
No, because AFAICT, bit 2 in the BAR would not be set.
> If it's not, how can I check?
Can you try 'lspci -vv' and look at the 'Region X:' lines which will
say 32 or 64-bit. I *think* that should reflect what actually got
written into the BARs.
Rob
^ permalink raw reply
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Leonardo Bras @ 2021-04-20 2:02 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <CAL_JsqK83MFqZ4yCz+i7sunpXFmi+vvjCSxVmcCh1YG=mOxY9A@mail.gmail.com>
On Mon, 2021-04-19 at 20:39 -0500, Rob Herring wrote:
> On Mon, Apr 19, 2021 at 7:35 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> >
> > On Mon, 2021-04-19 at 10:44 -0500, Rob Herring wrote:
> > > On Fri, Apr 16, 2021 at 3:58 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > > >
> > > > Hello Rob, thanks for this feedback!
> > > >
> > > > On Thu, 2021-04-15 at 13:59 -0500, Rob Herring wrote:
> > > > > +PPC and PCI lists
> > > > >
> > > > > On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > > > > >
> > > > > > Many other resource flag parsers already add this flag when the input
> > > > > > has bits 24 & 25 set, so update this one to do the same.
> > > > >
> > > > > Many others? Looks like sparc and powerpc to me.
> > > > >
> > > >
> > > > s390 also does that, but it look like it comes from a device-tree.
> > >
> > > I'm only looking at DT based platforms, and s390 doesn't use DT.
> >
> > Correct.
> > Sorry, I somehow write above the opposite of what I was thinking.
> >
> > >
> > > > > Those would be the
> > > > > ones I worry about breaking. Sparc doesn't use of/address.c so it's
> > > > > fine. Powerpc version of the flags code was only fixed in 2019, so I
> > > > > don't think powerpc will care either.
> > > >
> > > > In powerpc I reach this function with this stack, while configuring a
> > > > virtio-net device for a qemu/KVM pseries guest:
> > > >
> > > > pci_process_bridge_OF_ranges+0xac/0x2d4
> > > > pSeries_discover_phbs+0xc4/0x158
> > > > discover_phbs+0x40/0x60
> > > > do_one_initcall+0x60/0x2d0
> > > > kernel_init_freeable+0x308/0x3a8
> > > > kernel_init+0x2c/0x168
> > > > ret_from_kernel_thread+0x5c/0x70
> > > >
> > > > For this, both MMIO32 and MMIO64 resources will have flags 0x200.
> > >
> > > Oh good, powerpc has 2 possible flags parsing functions. So in the
> > > above path, do we need to set PCI_BASE_ADDRESS_MEM_TYPE_64?
> > >
> > > Does pci_parse_of_flags() get called in your case?
> > >
> >
> > It's called in some cases, but not for the device I am debugging
> > (virtio-net pci@800000020000000).
> >
> > For the above device, here is an expanded stack trace:
> >
> > of_bus_pci_get_flags() (from parser->bus->get_flags())
> > of_pci_range_parser_one() (from macro for_each_of_pci_range)
> > pci_process_bridge_OF_ranges+0xac/0x2d4
> > pSeries_discover_phbs+0xc4/0x158
> > discover_phbs+0x40/0x60
> > do_one_initcall+0x60/0x2d0
> > kernel_init_freeable+0x308/0x3a8
> > kernel_init+0x2c/0x168
> > ret_from_kernel_thread+0x5c/0x70
> >
> > For other devices, I could also see the following stack trace:
> > ## device ethernet@8
> >
> > pci_parse_of_flags()
> > of_create_pci_dev+0x7f0/0xa40
> > __of_scan_bus+0x248/0x320
> > pcibios_scan_phb+0x370/0x3b0
> > pcibios_init+0x8c/0x12c
> > do_one_initcall+0x60/0x2d0
> > kernel_init_freeable+0x308/0x3a8
> > kernel_init+0x2c/0x168
> > ret_from_kernel_thread+0x5c/0x70
> >
> > Devices that get parsed with of_bus_pci_get_flags() appears first at
> > dmesg (around 0.015s in my test), while devices that get parsed by
> > pci_parse_of_flags() appears later (0.025s in my test).
> >
> > I am not really used to this code, but having the term "discover phbs"
> > in the first trace and the term "scan phb" in the second, makes me
> > wonder if the first trace is seen on devices that are seen/described in
> > the device-tree and the second trace is seen in devices not present in
> > the device-tree and found scanning pci bus.
>
> That was my guess as well. I think on pSeries that most PCI devices
> are in the DT whereas on Arm and other flattened DT (non OpenFirmware)
> platforms PCI devices are not in DT.
>
It makes sense to me.
> Of course, for virtio devices,
> they would not be in DT in either case.
I don't get this part... in pseries it looks like virtio devices can be
in device-tree.
Oh, I think I get it... this pci@800000020000000 looks like a bus
(described in device-tree, so discovered), and then the devices are
inside it, getting scanned.
The virtio device gets the correct flags (from pci_parse_of_flags), but
the bus (pci@800000020000000) does not seem to get it correctly,
because it comes from of_bus_pci_get_flags() which makes sense
according to the name of the function.
(see lspci bellow, output without patch)
00:08.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev
01)
Subsystem: Red Hat, Inc. Device 1100
Device tree node:
/sys/firmware/devicetree/base/pci@800000020000000/ethernet@8
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR+ FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0
Interrupt: pin A routed to IRQ 19
IOMMU group: 0
Region 1: Memory at 200080020000 (32-bit, non-prefetchable)
[size=4K]
Region 4: Memory at 210000010000 (64-bit, prefetchable)
[size=16K]
Expansion ROM at 200080040000 [disabled] [size=256K]
Capabilities: [98] MSI-X: Enable+ Count=3 Masked-
Vector table: BAR=1 offset=00000000
PBA: BAR=1 offset=00000800
Capabilities: [84] Vendor Specific Information: VirtIO:
<unknown>
BAR=0 offset=00000000 size=00000000
Capabilities: [70] Vendor Specific Information: VirtIO: Notify
BAR=4 offset=00003000 size=00001000 multiplier=00000004
Capabilities: [60] Vendor Specific Information: VirtIO:
DeviceCfg
BAR=4 offset=00002000 size=00001000
Capabilities: [50] Vendor Specific Information: VirtIO: ISR
BAR=4 offset=00001000 size=00001000
Capabilities: [40] Vendor Specific Information: VirtIO:
CommonCfg
BAR=4 offset=00000000 size=00001000
Kernel driver in use: virtio-pci
>
> > > > > I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
> > > > > the flags. AFAICT, that's not set anywhere outside of arch code. So
> > > > > never for riscv, arm and arm64 at least. That leads me to
> > > > > pci_std_update_resource() which is where the PCI code sets BARs and
> > > > > just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
> > > > > IORESOURCE_* flags. So it seems like 64-bit is still not handled and
> > > > > neither is prefetch.
> > > > >
> > > >
> > > > I am not sure if you mean here:
> > > > a) it's ok to add IORESOURCE_MEM_64 here, because it does not affect
> > > > anything else, or
> > > > b) it should be using PCI_BASE_ADDRESS_MEM_TYPE_64
> > > > (or IORESOURCE_MEM_64 | PCI_BASE_ADDRESS_MEM_TYPE_64) instead, since
> > > > it's how it's added in powerpc/sparc, and else there is no point.
> > >
> > > I'm wondering if a) is incomplete and PCI_BASE_ADDRESS_MEM_TYPE_64
> > > also needs to be set. The question is ultimately are BARs getting set
> > > correctly for 64-bit? It looks to me like they aren't.
> >
> > I am not used to these terms, does BAR means 'Base Address Register'?
>
> Yes. Standard PCI thing.
Nice :)
>
> > If so, those are the addresses stored in pci->phb->mem_resources[i] and
> > pci->phb->mem_offset[i], printed from enable_ddw() (which takes place a
> > lot after discovering the device (0.17s in my run)).
> >
> > resource #1 pci@800000020000000: start=0x200080000000
> > end=0x2000ffffffff flags=0x200 desc=0x0 offset=0x200000000000
> > resource #2 pci@800000020000000: start=0x210000000000
> > end=0x21ffffffffff flags=0x200 desc=0x0 offset=0x0
> >
> > The message above was printed without this patch.
> > With the patch, the flags for memory resource #2 gets ORed with
> > 0x00100000.
>
> Right, as expected.
>
> > Is it enough to know if BARs are correctly set for 64-bit?
>
> No, because AFAICT, bit 2 in the BAR would not be set.
>
> > If it's not, how can I check?
>
> Can you try 'lspci -vv' and look at the 'Region X:' lines which will
> say 32 or 64-bit. I *think* that should reflect what actually got
> written into the BARs.
As seen in the lspci from above comment:
Region 1: Memory at 200080020000 (32-bit, non-prefetchable) [size=4K]
Region 4: Memory at 210000010000 (64-bit, prefetchable) [size=16K]
So it seems to be getting configured properly.
I think the point here is bus resources not getting the MEM_64 flag,
but device resources getting it correctly. Is that supposed to happen?
>
> Rob
Thanks Rob!
Leonardo Bras
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/8xx: Enhance readability of trap types
From: Xiongwei Song @ 2021-04-20 2:35 UTC (permalink / raw)
To: Christophe Leroy; +Cc: PowerPC, Paul Mackerras, Linux Kernel Mailing List
In-Reply-To: <e1147287bf6f2fb0693048fe8db0298c7870e419.1618847273.git.christophe.leroy@csgroup.eu>
On Mon, Apr 19, 2021 at 11:48 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> This patch makes use of trap types in head_8xx.S
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/interrupt.h | 29 ++++++++++++----
> arch/powerpc/kernel/head_8xx.S | 49 ++++++++++++++--------------
> 2 files changed, 47 insertions(+), 31 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index ed2c4042c3d1..cf2c5c3ae716 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -2,13 +2,6 @@
> #ifndef _ASM_POWERPC_INTERRUPT_H
> #define _ASM_POWERPC_INTERRUPT_H
>
> -#include <linux/context_tracking.h>
> -#include <linux/hardirq.h>
> -#include <asm/cputime.h>
> -#include <asm/ftrace.h>
> -#include <asm/kprobes.h>
> -#include <asm/runlatch.h>
> -
> /* BookE/4xx */
> #define INTERRUPT_CRITICAL_INPUT 0x100
>
> @@ -39,9 +32,11 @@
> /* BookE/BookS/4xx/8xx */
> #define INTERRUPT_DATA_STORAGE 0x300
> #define INTERRUPT_INST_STORAGE 0x400
> +#define INTERRUPT_EXTERNAL 0x500
> #define INTERRUPT_ALIGNMENT 0x600
> #define INTERRUPT_PROGRAM 0x700
> #define INTERRUPT_SYSCALL 0xc00
> +#define INTERRUPT_TRACE 0xd00
The INTERRUPT_TRACE macro is defined in BookS section.
In BookE, 0xd00 stands for debug interrupt, so I defined it as
INTERRUPT_DEBUG. I understand they are similar things,
but the terminologies are different in reference manuals.
Regards,
Xiongwei
^ permalink raw reply
* Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
From: Vineet Gupta @ 2021-04-20 2:48 UTC (permalink / raw)
To: Matthew Wilcox, brouer@redhat.com
Cc: arnd@kernel.org, grygorii.strashko@ti.com, netdev@vger.kernel.org,
ilias.apalodimas@linaro.org, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, mhocko@kernel.org,
linux-mm@kvack.org, mgorman@suse.de, mcroce@linux.microsoft.com,
linux-snps-arc@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
hch@lst.de, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20210417024522.GP2531743@casper.infradead.org>
Hi Matthew,
On 4/16/21 7:45 PM, Matthew Wilcox wrote:
> Replacement patch to fix compiler warning.
>
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> Date: Fri, 16 Apr 2021 16:34:55 -0400
> Subject: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
> To: brouer@redhat.com
> Cc: linux-kernel@vger.kernel.org,
> linux-mm@kvack.org,
> netdev@vger.kernel.org,
> linuxppc-dev@lists.ozlabs.org,
> linux-arm-kernel@lists.infradead.org,
> linux-mips@vger.kernel.org,
> ilias.apalodimas@linaro.org,
> mcroce@linux.microsoft.com,
> grygorii.strashko@ti.com,
> arnd@kernel.org,
> hch@lst.de,
> linux-snps-arc@lists.infradead.org,
> mhocko@kernel.org,
> mgorman@suse.de
>
> 32-bit architectures which expect 8-byte alignment for 8-byte integers
> and need 64-bit DMA addresses (arc, arm, mips, ppc) had their struct
> page inadvertently expanded in 2019.
FWIW, ARC doesn't require 8 byte alignment for 8 byte integers. This is
only needed for 8-byte atomics due to the requirements of LLOCKD/SCOND
instructions.
> When the dma_addr_t was added,
> it forced the alignment of the union to 8 bytes, which inserted a 4 byte
> gap between 'flags' and the union.
>
> Fix this by storing the dma_addr_t in one or two adjacent unsigned longs.
> This restores the alignment to that of an unsigned long, and also fixes a
> potential problem where (on a big endian platform), the bit used to denote
> PageTail could inadvertently get set, and a racing get_user_pages_fast()
> could dereference a bogus compound_head().
>
> Fixes: c25fff7171be ("mm: add dma_addr_t to struct page")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> include/linux/mm_types.h | 4 ++--
> include/net/page_pool.h | 12 +++++++++++-
> net/core/page_pool.c | 12 +++++++-----
> 3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 6613b26a8894..5aacc1c10a45 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -97,10 +97,10 @@ struct page {
> };
> struct { /* page_pool used by netstack */
> /**
> - * @dma_addr: might require a 64-bit value even on
> + * @dma_addr: might require a 64-bit value on
> * 32-bit architectures.
> */
> - dma_addr_t dma_addr;
> + unsigned long dma_addr[2];
> };
> struct { /* slab, slob and slub */
> union {
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index b5b195305346..ad6154dc206c 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -198,7 +198,17 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>
> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> {
> - return page->dma_addr;
> + dma_addr_t ret = page->dma_addr[0];
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + ret |= (dma_addr_t)page->dma_addr[1] << 16 << 16;
> + return ret;
> +}
> +
> +static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
> +{
> + page->dma_addr[0] = addr;
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + page->dma_addr[1] = addr >> 16 >> 16;
> }
>
> static inline bool is_page_pool_compiled_in(void)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index ad8b0707af04..f014fd8c19a6 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -174,8 +174,10 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool,
> struct page *page,
> unsigned int dma_sync_size)
> {
> + dma_addr_t dma_addr = page_pool_get_dma_addr(page);
> +
> dma_sync_size = min(dma_sync_size, pool->p.max_len);
> - dma_sync_single_range_for_device(pool->p.dev, page->dma_addr,
> + dma_sync_single_range_for_device(pool->p.dev, dma_addr,
> pool->p.offset, dma_sync_size,
> pool->p.dma_dir);
> }
> @@ -226,7 +228,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
> put_page(page);
> return NULL;
> }
> - page->dma_addr = dma;
> + page_pool_set_dma_addr(page, dma);
>
> if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
> page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
> @@ -294,13 +296,13 @@ void page_pool_release_page(struct page_pool *pool, struct page *page)
> */
> goto skip_dma_unmap;
>
> - dma = page->dma_addr;
> + dma = page_pool_get_dma_addr(page);
>
> - /* When page is unmapped, it cannot be returned our pool */
> + /* When page is unmapped, it cannot be returned to our pool */
> dma_unmap_page_attrs(pool->p.dev, dma,
> PAGE_SIZE << pool->p.order, pool->p.dma_dir,
> DMA_ATTR_SKIP_CPU_SYNC);
> - page->dma_addr = 0;
> + page_pool_set_dma_addr(page, 0);
> skip_dma_unmap:
> /* This may be the last page returned, releasing the pool, so
> * it is not safe to reference pool afterwards.
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 40f5c8e99b3f2f53db08055f415af2aac416360e
From: kernel test robot @ 2021-04-20 2:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 40f5c8e99b3f2f53db08055f415af2aac416360e Automatic merge of 'master' into merge (2021-04-19 12:37)
elapsed time: 1430m
configs tested: 131
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
x86_64 allyesconfig
riscv allmodconfig
i386 allyesconfig
riscv allyesconfig
powerpc chrp32_defconfig
powerpc ppc6xx_defconfig
arm mini2440_defconfig
arm lpc32xx_defconfig
mips tb0226_defconfig
arm mvebu_v5_defconfig
arm realview_defconfig
powerpc pseries_defconfig
arm pxa255-idp_defconfig
arm versatile_defconfig
arm multi_v4t_defconfig
powerpc motionpro_defconfig
sh ap325rxa_defconfig
nds32 allnoconfig
mips pic32mzda_defconfig
arm64 alldefconfig
powerpc fsp2_defconfig
sh dreamcast_defconfig
powerpc ppa8548_defconfig
xtensa cadence_csp_defconfig
sh se7750_defconfig
sh apsh4a3a_defconfig
sparc sparc32_defconfig
um alldefconfig
m68k m5272c3_defconfig
mips gcw0_defconfig
arm assabet_defconfig
powerpc pq2fads_defconfig
m68k m5475evb_defconfig
arc axs103_smp_defconfig
mips malta_kvm_defconfig
riscv alldefconfig
powerpc mpc85xx_cds_defconfig
ia64 defconfig
sparc defconfig
powerpc ppc64e_defconfig
powerpc wii_defconfig
mips e55_defconfig
mips tb0287_defconfig
powerpc powernv_defconfig
m68k m5407c3_defconfig
arm lubbock_defconfig
powerpc ppc64_defconfig
m68k amcore_defconfig
mips jmr3927_defconfig
mips capcella_defconfig
powerpc warp_defconfig
mips bcm47xx_defconfig
alpha allyesconfig
sh se7751_defconfig
m68k m5275evb_defconfig
sh sh7785lcr_defconfig
powerpc icon_defconfig
arm gemini_defconfig
arm exynos_defconfig
powerpc kmeter1_defconfig
powerpc mpc834x_mds_defconfig
riscv rv32_defconfig
ia64 allmodconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
sparc allyesconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a003-20210419
x86_64 randconfig-a001-20210419
x86_64 randconfig-a005-20210419
x86_64 randconfig-a002-20210419
x86_64 randconfig-a006-20210419
x86_64 randconfig-a004-20210419
i386 randconfig-a003-20210419
i386 randconfig-a001-20210419
i386 randconfig-a006-20210419
i386 randconfig-a005-20210419
i386 randconfig-a004-20210419
i386 randconfig-a002-20210419
i386 randconfig-a015-20210419
i386 randconfig-a013-20210419
i386 randconfig-a014-20210419
i386 randconfig-a016-20210419
i386 randconfig-a012-20210419
i386 randconfig-a011-20210419
riscv nommu_k210_defconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
um allmodconfig
um allnoconfig
um allyesconfig
um defconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-a014-20210419
x86_64 randconfig-a015-20210419
x86_64 randconfig-a013-20210419
x86_64 randconfig-a011-20210419
x86_64 randconfig-a012-20210419
x86_64 randconfig-a016-20210419
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:next] BUILD SUCCESS cbd3d5ba46b68c033986a6087209930f001cbcca
From: kernel test robot @ 2021-04-20 2:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
branch HEAD: cbd3d5ba46b68c033986a6087209930f001cbcca powerpc/fadump: Fix compile error since trap type change
elapsed time: 725m
configs tested: 132
configs skipped: 5
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
x86_64 allyesconfig
riscv allmodconfig
i386 allyesconfig
riscv allyesconfig
m68k atari_defconfig
arc vdk_hs38_smp_defconfig
powerpc kmeter1_defconfig
openrisc simple_smp_defconfig
powerpc mpc5200_defconfig
powerpc motionpro_defconfig
sh ap325rxa_defconfig
nds32 allnoconfig
mips pic32mzda_defconfig
sh rts7751r2d1_defconfig
xtensa virt_defconfig
arm mmp2_defconfig
arm omap1_defconfig
arm multi_v5_defconfig
powerpc tqm8540_defconfig
arm64 alldefconfig
powerpc fsp2_defconfig
sh dreamcast_defconfig
powerpc ppa8548_defconfig
xtensa cadence_csp_defconfig
sh se7750_defconfig
sh apsh4a3a_defconfig
sparc sparc32_defconfig
um alldefconfig
m68k m5272c3_defconfig
um allnoconfig
powerpc ep88xc_defconfig
powerpc socrates_defconfig
sparc sparc64_defconfig
arm magician_defconfig
m68k mac_defconfig
powerpc linkstation_defconfig
sh sdk7786_defconfig
arm vexpress_defconfig
mips fuloong2e_defconfig
parisc generic-64bit_defconfig
mips maltaup_defconfig
arm h5000_defconfig
powerpc mpc83xx_defconfig
m68k amcore_defconfig
mips jmr3927_defconfig
mips capcella_defconfig
powerpc warp_defconfig
alpha allyesconfig
mips bcm47xx_defconfig
mips mpc30x_defconfig
powerpc ppc64_defconfig
powerpc kilauea_defconfig
mips maltaaprp_defconfig
arm spear3xx_defconfig
sh se7751_defconfig
m68k m5275evb_defconfig
sh sh7785lcr_defconfig
powerpc icon_defconfig
arm gemini_defconfig
arm exynos_defconfig
powerpc mpc834x_mds_defconfig
riscv rv32_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a003-20210419
x86_64 randconfig-a001-20210419
x86_64 randconfig-a005-20210419
x86_64 randconfig-a002-20210419
x86_64 randconfig-a006-20210419
x86_64 randconfig-a004-20210419
i386 randconfig-a003-20210419
i386 randconfig-a001-20210419
i386 randconfig-a006-20210419
i386 randconfig-a005-20210419
i386 randconfig-a004-20210419
i386 randconfig-a002-20210419
i386 randconfig-a015-20210419
i386 randconfig-a013-20210419
i386 randconfig-a014-20210419
i386 randconfig-a016-20210419
i386 randconfig-a012-20210419
i386 randconfig-a011-20210419
riscv nommu_k210_defconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
um allmodconfig
um allyesconfig
um defconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-a014-20210419
x86_64 randconfig-a015-20210419
x86_64 randconfig-a013-20210419
x86_64 randconfig-a011-20210419
x86_64 randconfig-a012-20210419
x86_64 randconfig-a016-20210419
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [PATCH V2 0/1] powerpc/perf: Clear pending PMI in ppmu callbacks
From: Athira Rajeev @ 2021-04-20 3:01 UTC (permalink / raw)
To: mpe; +Cc: nasastry, maddy, linuxppc-dev, npiggin
Running perf fuzzer testsuite popped up below messages
in the dmesg logs:
"Can't find PMC that caused IRQ"
This means a PMU exception happened, but none of the PMC's (Performance
Monitor Counter) were found to be overflown. Perf interrupt handler checks
the PMC's to see which PMC has overflown and if none of the PMCs are
overflown ( counter value not >= 0x80000000 ), it throws warning:
"Can't find PMC that caused IRQ".
Powerpc has capability to mask and replay a performance monitoring
interrupt (PMI). In case of replayed PMI, there are some corner cases
that clears the PMCs after masking. In such cases, the perf interrupt
handler will not find the active PMC values that had caused the overflow
and thus leading to this message. This patchset attempts to fix those
corner cases.
However there is one more case in PowerNV where these messages are
emitted during system wide profiling or when a specific CPU is monitored
for an event. That is, when a counter overflow just before entering idle
and a PMI gets triggered after wakeup from idle. Since PMCs
are not saved in the idle path, perf interrupt handler will not
find overflown counter value and emits the "Can't find PMC" messages.
This patch documents this race condition in powerpc core-book3s.
Patch fixes the ppmu callbacks to disable pending interrupt before clearing
the overflown PMC and documents the race condition in idle path.
Changelog:
Changes from v1 -> v2
Addressed review comments from Nicholas Piggin
- Moved the PMI pending check and clearing function
to arch/powerpc/include/asm/hw_irq.h and renamed
function to "get_clear_pmi_irq_pending"
- Along with checking for pending PMI bit in Paca,
look for PMAO bit in MMCR0 register to decide on
pending PMI interrupt.
Athira Rajeev (1):
powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting
an overflown PMC
arch/powerpc/include/asm/hw_irq.h | 19 ++++++++
arch/powerpc/perf/core-book3s.c | 77 +++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+)
--
2.26.2
^ permalink raw reply
* [PATCH V2 1/1] powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
From: Athira Rajeev @ 2021-04-20 3:01 UTC (permalink / raw)
To: mpe; +Cc: nasastry, maddy, linuxppc-dev, npiggin
In-Reply-To: <1618887697-1556-1-git-send-email-atrajeev@linux.vnet.ibm.com>
Running perf fuzzer showed below in dmesg logs:
"Can't find PMC that caused IRQ"
This means a PMU exception happened, but none of the PMC's (Performance
Monitor Counter) were found to be overflown. There are some corner cases
that clears the PMCs after PMI gets masked. In such cases, the perf
interrupt handler will not find the active PMC values that had caused
the overflow and thus leads to this message while replaying.
Case 1: PMU Interrupt happens during replay of other interrupts and
counter values gets cleared by PMU callbacks before replay:
During replay of interrupts like timer, __do_irq and doorbell exception, we
conditionally enable interrupts via may_hard_irq_enable(). This could
potentially create a window to generate a PMI. Since irq soft mask is set
to ALL_DISABLED, the PMI will get masked here. We could get IPIs run before
perf interrupt is replayed and the PMU events could deleted or stopped.
This will change the PMU SPR values and resets the counters. Snippet of
ftrace log showing PMU callbacks invoked in "__do_irq":
<idle>-0 [051] dns. 132025441306354: __do_irq <-call_do_irq
<idle>-0 [051] dns. 132025441306430: irq_enter <-__do_irq
<idle>-0 [051] dns. 132025441306503: irq_enter_rcu <-__do_irq
<idle>-0 [051] dnH. 132025441306599: xive_get_irq <-__do_irq
<<>>
<idle>-0 [051] dnH. 132025441307770: generic_smp_call_function_single_interrupt <-smp_ipi_demux_relaxed
<idle>-0 [051] dnH. 132025441307839: flush_smp_call_function_queue <-smp_ipi_demux_relaxed
<idle>-0 [051] dnH. 132025441308057: _raw_spin_lock <-event_function
<idle>-0 [051] dnH. 132025441308206: power_pmu_disable <-perf_pmu_disable
<idle>-0 [051] dnH. 132025441308337: power_pmu_del <-event_sched_out
<idle>-0 [051] dnH. 132025441308407: power_pmu_read <-power_pmu_del
<idle>-0 [051] dnH. 132025441308477: read_pmc <-power_pmu_read
<idle>-0 [051] dnH. 132025441308590: isa207_disable_pmc <-power_pmu_del
<idle>-0 [051] dnH. 132025441308663: write_pmc <-power_pmu_del
<idle>-0 [051] dnH. 132025441308787: power_pmu_event_idx <-perf_event_update_userpage
<idle>-0 [051] dnH. 132025441308859: rcu_read_unlock_strict <-perf_event_update_userpage
<idle>-0 [051] dnH. 132025441308975: power_pmu_enable <-perf_pmu_enable
<<>>
<idle>-0 [051] dnH. 132025441311108: irq_exit <-__do_irq
<idle>-0 [051] dns. 132025441311319: performance_monitor_exception <-replay_soft_interrupts
Case 2: PMI's masked during local_* operations, example local_add.
If the local_add operation happens within a local_irq_save, replay of
PMI will be during local_irq_restore. Similar to case 1, this could
also create a window before replay where PMU events gets deleted or
stopped.
Patch adds a fix to update the PMU callback functions (del,stop,enable) to
check for pending perf interrupt. If there is an overflown PMC and pending
perf interrupt indicated in Paca or by PMAO bit set in MMCR0, clear the PMI
bit in paca to drop that sample. Also clear the MMCR0 PMAO bit which
otherwise could lead to spurious interrupts in some corner cases. Example,
a timer after power_pmu_del which will re-enable interrupts since PMI is
cleared and triggers a PMI again since PMAO bit is still set. Another
condition occures if had disabled MSR[EE] right before perf interrupt
came in. Re-enabling interrupt will trigger PMI since PMAO is still set.
But fails to find valid overflow if PMC get cleared before enabling EE.
We can't just replay PMI any time. Hence this approach is preferred rather
than replaying PMI before resetting overflown PMC. Patch also documents
core-book3s on a race condition which can trigger these PMC messages during
idle path in PowerNV.
Fixes: f442d004806e ("powerpc/64s: Add support to mask perf interrupts and replay them")
Reported-by: Nageswara R Sastry <nasastry@in.ibm.com>
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/hw_irq.h | 19 ++++++++
arch/powerpc/perf/core-book3s.c | 77 +++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+)
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index 56a98936a6a9..7e192bd8253b 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -215,6 +215,23 @@ static inline bool arch_irqs_disabled(void)
return arch_irqs_disabled_flags(arch_local_save_flags());
}
+static inline int get_clear_pmi_irq_pending(void)
+{
+ /*
+ * Some corner cases could clear the PMU counter overflow
+ * while a masked PMI is pending. One of such case is
+ * when a PMI happens during interrupt replay and perf
+ * counter values gets cleared by PMU callbacks before
+ * replay. So the pending PMI must be cleared here.
+ */
+ if (get_paca()->irq_happened & PACA_IRQ_PMI) {
+ WARN_ON_ONCE(mfmsr() & MSR_EE);
+ get_paca()->irq_happened &= ~PACA_IRQ_PMI;
+ return 1;
+ }
+ return 0;
+}
+
#ifdef CONFIG_PPC_BOOK3S
/*
* To support disabling and enabling of irq with PMI, set of
@@ -391,6 +408,8 @@ static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
static inline void may_hard_irq_enable(void) { }
+static inline int get_clear_pmi_irq_pending(void) { return 0; }
+
static inline void irq_soft_mask_regs_set_state(struct pt_regs *regs, unsigned long val)
{
}
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 766f064f00fb..6ddac1b913fa 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -847,6 +847,20 @@ static void write_pmc(int idx, unsigned long val)
}
}
+static int pmc_overflown(int idx)
+{
+ unsigned long val[8];
+ int i;
+
+ for (i = 0; i < ppmu->n_counter; i++)
+ val[i] = read_pmc(i + 1);
+
+ if ((int)val[idx-1] < 0)
+ return 1;
+
+ return 0;
+}
+
/* Called from sysrq_handle_showregs() */
void perf_event_print_debug(void)
{
@@ -1438,6 +1452,17 @@ static void power_pmu_enable(struct pmu *pmu)
event = cpuhw->event[i];
if (event->hw.idx && event->hw.idx != hwc_index[i] + 1) {
power_pmu_read(event);
+ /*
+ * if the PMC corresponding to event->hw.idx is
+ * overflown, check if there is any pending perf
+ * interrupt set in paca. If so, disable the interrupt
+ * by clearing the paca bit for PMI since we are going
+ * to reset the PMC. power_pmu_enable will reset PMAO bit
+ * of MMCR0 while enabling the event. So PMAO check
+ * is not needed here (versus PMAO check done in del/stop).
+ */
+ if (pmc_overflown(event->hw.idx))
+ get_clear_pmi_irq_pending();
write_pmc(event->hw.idx, 0);
event->hw.idx = 0;
}
@@ -1474,6 +1499,10 @@ static void power_pmu_enable(struct pmu *pmu)
event->hw.idx = idx;
if (event->hw.state & PERF_HES_STOPPED)
val = 0;
+
+ /* See above for get_clear_pmi_irq_pending */
+ if (pmc_overflown(event->hw.idx))
+ get_clear_pmi_irq_pending();
write_pmc(idx, val);
perf_event_update_userpage(event);
@@ -1619,6 +1648,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
struct cpu_hw_events *cpuhw;
long i;
unsigned long flags;
+ unsigned long val_mmcr0;
local_irq_save(flags);
perf_pmu_disable(event->pmu);
@@ -1636,6 +1666,23 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
--cpuhw->n_events;
ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
if (event->hw.idx) {
+ /*
+ * if the PMC corresponding to event->hw.idx is
+ * overflown, check if there is any pending perf
+ * interrupt set in paca or indicated by PMAO bit
+ * in MMCR0. If so, disable the interrupt and clear
+ * the MMCR0 PMAO bit since we are going to reset
+ * the PMC and delete the event.
+ */
+ if (pmc_overflown(event->hw.idx)) {
+ if ((get_clear_pmi_irq_pending() | (mfspr(SPRN_MMCR0) & MMCR0_PMAO))) {
+ val_mmcr0 = mfspr(SPRN_MMCR0);
+ val_mmcr0 &= ~MMCR0_PMAO;
+ write_mmcr0(cpuhw, val_mmcr0);
+ mb();
+ isync();
+ }
+ }
write_pmc(event->hw.idx, 0);
event->hw.idx = 0;
}
@@ -1705,6 +1752,8 @@ static void power_pmu_start(struct perf_event *event, int ef_flags)
static void power_pmu_stop(struct perf_event *event, int ef_flags)
{
unsigned long flags;
+ unsigned long val_mmcr0;
+ struct cpu_hw_events *cpuhw;
if (!event->hw.idx || !event->hw.sample_period)
return;
@@ -1713,8 +1762,27 @@ static void power_pmu_stop(struct perf_event *event, int ef_flags)
return;
local_irq_save(flags);
+ cpuhw = this_cpu_ptr(&cpu_hw_events);
perf_pmu_disable(event->pmu);
+ /*
+ * if the PMC corresponding to event->hw.idx is
+ * overflown, check if there is any pending perf
+ * interrupt set in paca or indicated by PMAO bit
+ * in MMCR0. If so, disable the interrupt and clear
+ * the MMCR0 PMAO bit since we are going to reset
+ * the PMC.
+ */
+ if (pmc_overflown(event->hw.idx)) {
+ if ((get_clear_pmi_irq_pending() | (mfspr(SPRN_MMCR0) & MMCR0_PMAO))) {
+ val_mmcr0 = mfspr(SPRN_MMCR0);
+ val_mmcr0 &= ~MMCR0_PMAO;
+ write_mmcr0(cpuhw, val_mmcr0);
+ mb();
+ isync();
+ }
+ }
+
power_pmu_read(event);
event->hw.state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
write_pmc(event->hw.idx, 0);
@@ -2343,6 +2411,15 @@ static void __perf_event_interrupt(struct pt_regs *regs)
}
}
}
+
+ /*
+ * During system wide profling or while specific CPU
+ * is monitored for an event, some corner cases could
+ * cause PMC to overflow in idle path. This will trigger
+ * a PMI after waking up from idle. Since counter values
+ * are _not_ saved/restored in idle path, can lead to
+ * below "Can't find PMC" message.
+ */
if (unlikely(!found) && !arch_irq_disabled_regs(regs))
printk_ratelimited(KERN_WARNING "Can't find PMC that caused IRQ\n");
--
2.26.2
^ permalink raw reply related
* Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
From: Matthew Wilcox @ 2021-04-20 3:10 UTC (permalink / raw)
To: Vineet Gupta
Cc: arnd@kernel.org, grygorii.strashko@ti.com, netdev@vger.kernel.org,
ilias.apalodimas@linaro.org, linux-kernel@vger.kernel.org,
linux-mips@vger.kernel.org, mhocko@kernel.org, linux-mm@kvack.org,
mgorman@suse.de, brouer@redhat.com, mcroce@linux.microsoft.com,
linux-snps-arc@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
hch@lst.de, linux-arm-kernel@lists.infradead.org
In-Reply-To: <9f99b0a0-f1c1-f3b0-5f84-3a4bfc711725@synopsys.com>
On Tue, Apr 20, 2021 at 02:48:17AM +0000, Vineet Gupta wrote:
> > 32-bit architectures which expect 8-byte alignment for 8-byte integers
> > and need 64-bit DMA addresses (arc, arm, mips, ppc) had their struct
> > page inadvertently expanded in 2019.
>
> FWIW, ARC doesn't require 8 byte alignment for 8 byte integers. This is
> only needed for 8-byte atomics due to the requirements of LLOCKD/SCOND
> instructions.
Ah, like x86? OK, great, I'll drop your arch from the list of
affected. Thanks!
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Add shutdown() to vio_driver and vio_bus
From: Michael Ellerman @ 2021-04-20 3:42 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <59bd8028-cb1a-fdf6-74ce-68e868e4f486@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> On 4/17/21 5:30 AM, Michael Ellerman wrote:
>> Tyrel Datwyler <tyreld@linux.ibm.com> writes:
>>> On 4/1/21 5:13 PM, Tyrel Datwyler wrote:
>>>> Currently, neither the vio_bus or vio_driver structures provide support
>>>> for a shutdown() routine.
>>>>
>>>> Add support for shutdown() by allowing drivers to provide a
>>>> implementation via function pointer in their vio_driver struct and
>>>> provide a proper implementation in the driver template for the vio_bus
>>>> that calls a vio drivers shutdown() if defined.
>>>>
>>>> In the case that no shutdown() is defined by a vio driver and a kexec is
>>>> in progress we implement a big hammer that calls remove() to ensure no
>>>> further DMA for the devices is possible.
>>>>
>>>> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
>>>> ---
>>>
>>> Ping... any comments, problems with this approach?
>>
>> The kexec part seems like a bit of a hack.
>>
>> It also doesn't help for kdump, when none of the shutdown code is run.
>
> If I understand correctly for kdump we have a reserved memory space where the
> kdump kernel is loaded, but for kexec the memory region isn't reserved ahead of
> time meaning we can try and load the kernel over potential memory used for DMA
> by the current kernel.
That's correct.
>> How many drivers do we have? Can we just implement a proper shutdown for
>> them?
>
> Well that is the end goal. I just don't currently have the bandwidth to do each
> driver myself with a proper shutdown sequence, and thought this was a launching
> off point to at least introduce the shutdown callback to the VIO bus.
Fair enough.
> Off the top of my head we have 3 storage drivers, 2 network drivers, vtpm, vmc,
> pseries_rng, nx, nx842, hvcs, hvc_vio.
>
> I can drop the kexec_in_progress hammer and just have each driver call remove()
> themselves in their shutdown function. Leave it to each maintainer to decide if
> remove() is enough or if there is a more lightweight quiesce sequence they
> choose to implement.
That's OK, you've convinced me. I'll take it as-is.
Eventually it would be good for drivers to implement shutdown in the
optimal way for their device, but that can be done incrementally.
cheers
^ permalink raw reply
* Re: [PATCH v4 4/9] powerpc/mm/book3s64: Fix possible build error
From: Michael Ellerman @ 2021-04-20 3:43 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: Aneesh Kumar K.V, npiggin, kaleshsingh, joel, linuxppc-dev
In-Reply-To: <20210414085915.301189-5-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Update _tlbiel_pid() such that we can avoid build errors like below when
> using this function in other places.
>
> arch/powerpc/mm/book3s64/radix_tlb.c: In function ‘__radix__flush_tlb_range_psize’:
> arch/powerpc/mm/book3s64/radix_tlb.c:114:2: warning: ‘asm’ operand 3 probably does not match constraints
> 114 | asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
> | ^~~
> arch/powerpc/mm/book3s64/radix_tlb.c:114:2: error: impossible constraint in ‘asm’
> make[4]: *** [scripts/Makefile.build:271: arch/powerpc/mm/book3s64/radix_tlb.o] Error 1
> m
>
> With this fix, we can also drop the __always_inline in __radix_flush_tlb_range_psize
> which was added by commit e12d6d7d46a6 ("powerpc/mm/radix: mark __radix__flush_tlb_range_psize() as __always_inline")
>
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/radix_tlb.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 409e61210789..817a02ef6032 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -291,22 +291,30 @@ static inline void fixup_tlbie_lpid(unsigned long lpid)
> /*
> * We use 128 set in radix mode and 256 set in hpt mode.
> */
> -static __always_inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
> +static inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
> {
> int set;
>
> asm volatile("ptesync": : :"memory");
>
> - /*
> - * Flush the first set of the TLB, and if we're doing a RIC_FLUSH_ALL,
> - * also flush the entire Page Walk Cache.
> - */
> - __tlbiel_pid(pid, 0, ric);
> + switch (ric) {
> + case RIC_FLUSH_PWC:
>
> - /* For PWC, only one flush is needed */
> - if (ric == RIC_FLUSH_PWC) {
> + /* For PWC, only one flush is needed */
> + __tlbiel_pid(pid, 0, RIC_FLUSH_PWC);
> ppc_after_tlbiel_barrier();
> return;
> + case RIC_FLUSH_TLB:
> + __tlbiel_pid(pid, 0, RIC_FLUSH_TLB);
> + break;
> + case RIC_FLUSH_ALL:
> + default:
> + /*
> + * Flush the first set of the TLB, and if
> + * we're doing a RIC_FLUSH_ALL, also flush
> + * the entire Page Walk Cache.
> + */
> + __tlbiel_pid(pid, 0, RIC_FLUSH_ALL);
> }
>
> if (!cpu_has_feature(CPU_FTR_ARCH_31)) {
> @@ -1176,7 +1184,7 @@ void radix__tlb_flush(struct mmu_gather *tlb)
> }
> }
>
> -static __always_inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
> +static void __radix__flush_tlb_range_psize(struct mm_struct *mm,
> unsigned long start, unsigned long end,
> int psize, bool also_pwc)
> {
> --
> 2.30.2
^ permalink raw reply
* Re: [PATCH v4 6/9] mm/mremap: Use range flush that does TLB and page walk cache flush
From: Michael Ellerman @ 2021-04-20 3:47 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: joel, Aneesh Kumar K.V, linuxppc-dev, npiggin, kaleshsingh
In-Reply-To: <20210414085915.301189-7-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Some architectures do have the concept of page walk cache which need
> to be flush when updating higher levels of page tables. A fast mremap
> that involves moving page table pages instead of copying pte entries
> should flush page walk cache since the old translation cache is no more
> valid.
>
> Add new helper flush_pte_tlb_pwc_range() which invalidates both TLB and
> page walk cache where TLB entries are mapped with page size PAGE_SIZE.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/include/asm/book3s/64/tlbflush.h | 11 +++++++++++
> mm/mremap.c | 15 +++++++++++++--
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> index f9f8a3a264f7..c236b66f490b 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> @@ -80,6 +80,17 @@ static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> return flush_hugetlb_tlb_pwc_range(vma, start, end, false);
> }
>
> +#define flush_pte_tlb_pwc_range flush_tlb_pwc_range
> +static inline void flush_pte_tlb_pwc_range(struct vm_area_struct *vma,
> + unsigned long start, unsigned long end,
> + bool also_pwc)
This still uses the also_pwc name, which is a bit inconsistent with the
previous patch.
But, does it even need to be a parameter? AFAICS you always pass true,
and pwc=true is sort of implied by the name isn't it?
cheers
> +{
> + if (radix_enabled())
> + return radix__flush_tlb_pwc_range_psize(vma->vm_mm, start,
> + end, mmu_virtual_psize, also_pwc);
> + return hash__flush_tlb_range(vma, start, end);
> +}
> +
> static inline void flush_tlb_range(struct vm_area_struct *vma,
> unsigned long start, unsigned long end)
> {
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 574287f9bb39..0e7b11daafee 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -210,6 +210,17 @@ static void move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd,
> drop_rmap_locks(vma);
> }
>
> +#ifndef flush_pte_tlb_pwc_range
> +#define flush_pte_tlb_pwc_range flush_pte_tlb_pwc_range
> +static inline void flush_pte_tlb_pwc_range(struct vm_area_struct *vma,
> + unsigned long start,
> + unsigned long end,
> + bool also_pwc)
> +{
> + return flush_tlb_range(vma, start, end);
> +}
> +#endif
> +
> #ifdef CONFIG_HAVE_MOVE_PMD
> static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr,
> unsigned long new_addr, pmd_t *old_pmd, pmd_t *new_pmd)
> @@ -260,7 +271,7 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr,
> VM_BUG_ON(!pmd_none(*new_pmd));
> pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd));
>
> - flush_tlb_range(vma, old_addr, old_addr + PMD_SIZE);
> + flush_pte_tlb_pwc_range(vma, old_addr, old_addr + PMD_SIZE, true);
> if (new_ptl != old_ptl)
> spin_unlock(new_ptl);
> spin_unlock(old_ptl);
> @@ -307,7 +318,7 @@ static bool move_normal_pud(struct vm_area_struct *vma, unsigned long old_addr,
> VM_BUG_ON(!pud_none(*new_pud));
>
> pud_populate(mm, new_pud, (pmd_t *)pud_page_vaddr(pud));
> - flush_tlb_range(vma, old_addr, old_addr + PUD_SIZE);
> + flush_pte_tlb_pwc_range(vma, old_addr, old_addr + PUD_SIZE, true);
> if (new_ptl != old_ptl)
> spin_unlock(new_ptl);
> spin_unlock(old_ptl);
> --
> 2.30.2
^ permalink raw reply
* Re: [PATCH v4 8/9] mm/mremap: Allow arch runtime override
From: Michael Ellerman @ 2021-04-20 3:52 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: joel, Aneesh Kumar K.V, linuxppc-dev, npiggin, kaleshsingh
In-Reply-To: <20210414085915.301189-9-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Architectures like ppc64 support faster mremap only with radix
> translation. Hence allow a runtime check w.r.t support for fast mremap.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/include/asm/tlb.h | 6 ++++++
> mm/mremap.c | 15 ++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
> index 160422a439aa..058918a7cd3c 100644
> --- a/arch/powerpc/include/asm/tlb.h
> +++ b/arch/powerpc/include/asm/tlb.h
> @@ -83,5 +83,11 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
> }
> #endif
>
> +#define arch_supports_page_tables_move arch_supports_page_tables_move
> +static inline bool arch_supports_page_tables_move(void)
> +{
> + return radix_enabled();
> +}
Not sure it's worth a respin on its own, but page table*s* move is
slightly strange phrasing.
arch_supports_move_page_tables() or arch_supports_page_table_move()
would be more typical.
cheers
^ permalink raw reply
* Re: [PATCH 2/2] hotplug-cpu.c: set UNISOLATE on dlpar_cpu_remove() failure
From: Michael Ellerman @ 2021-04-20 3:57 UTC (permalink / raw)
To: Daniel Henrique Barboza, linuxppc-dev; +Cc: david
In-Reply-To: <f4d65cf6-08d1-0bcc-cba2-845099a82aa4@gmail.com>
Daniel Henrique Barboza <danielhb413@gmail.com> writes:
> On 4/19/21 9:48 AM, Michael Ellerman wrote:
>> Daniel Henrique Barboza <danielhb413@gmail.com> writes:
>>> The RTAS set-indicator call, when attempting to UNISOLATE a DRC that is
>>> already UNISOLATED or CONFIGURED, returns RTAS_OK and does nothing else
>>> for both QEMU and phyp. This gives us an opportunity to use this
>>> behavior to signal the hypervisor layer when an error during device
>>> removal happens, allowing it to do a proper error handling, while not
>>> breaking QEMU/phyp implementations that don't have this support.
>>>
>>> This patch introduces this idea by unisolating all CPU DRCs that failed
>>> to be removed by dlpar_cpu_remove_by_index(), when handling the
>>> PSERIES_HP_ELOG_ID_DRC_INDEX event. This is being done for this event
>>> only because its the only CPU removal event QEMU uses, and there's no
>>> need at this moment to add this mechanism for phyp only code.
>>
>> Have you also confirmed that phyp is not bothered by it? ie. everything
>> seems to continue working when you trigger this path on phyp.
>
> Yes. Daniel Bueso (dbuesom@us.ibm.com) from the partition firmware team
> helped me with that. We confirmed that phyp returns RTAS_OK under these
> conditions (Unisolating an unisolated/configured DRC).
Thanks.
cheers
^ permalink raw reply
* Re: [PATCH 0/2] Change struct page layout for page_pool
From: Michael Ellerman @ 2021-04-20 4:03 UTC (permalink / raw)
To: Matthew Wilcox (Oracle), brouer
Cc: arnd, grygorii.strashko, linux-snps-arc, netdev, ilias.apalodimas,
linux-kernel, Matthew Wilcox (Oracle), linux-mips, linux-mm,
mgorman, mcroce, mhocko, linuxppc-dev, hch, linux-arm-kernel
In-Reply-To: <20210416230724.2519198-1-willy@infradead.org>
"Matthew Wilcox (Oracle)" <willy@infradead.org> writes:
> The first patch here fixes two bugs on ppc32, and mips32. It fixes one
> bug on arc and arm32 (in certain configurations). It probably makes
> sense to get it in ASAP through the networking tree. I'd like to see
> testing on those four architectures if possible?
Sorry I don't have easy access to any hardware that fits the bill. At
some point I'll be able to go to the office and setup a machine that (I
think) can test these paths, but I don't have an ETA on that.
You and others seem to have done lots of analysis on this though, so I
think you should merge the fixes without waiting on ppc32 testing.
cheers
>
> The second patch enables new functionality. It is much less urgent.
> I'd really like to see Mel & Michal's thoughts on it.
>
> I have only compile-tested these patches.
>
> Matthew Wilcox (Oracle) (2):
> mm: Fix struct page layout on 32-bit systems
> mm: Indicate pfmemalloc pages in compound_head
>
> include/linux/mm.h | 12 +++++++-----
> include/linux/mm_types.h | 9 ++++-----
> include/net/page_pool.h | 12 +++++++++++-
> net/core/page_pool.c | 12 +++++++-----
> 4 files changed, 29 insertions(+), 16 deletions(-)
>
> --
> 2.30.2
^ permalink raw reply
* Re: [PATCH v4 6/9] mm/mremap: Use range flush that does TLB and page walk cache flush
From: Aneesh Kumar K.V @ 2021-04-20 4:17 UTC (permalink / raw)
To: Michael Ellerman, linux-mm, akpm; +Cc: joel, linuxppc-dev, npiggin, kaleshsingh
In-Reply-To: <87fszld3bt.fsf@mpe.ellerman.id.au>
On 4/20/21 9:17 AM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Some architectures do have the concept of page walk cache which need
>> to be flush when updating higher levels of page tables. A fast mremap
>> that involves moving page table pages instead of copying pte entries
>> should flush page walk cache since the old translation cache is no more
>> valid.
>>
>> Add new helper flush_pte_tlb_pwc_range() which invalidates both TLB and
>> page walk cache where TLB entries are mapped with page size PAGE_SIZE.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/book3s/64/tlbflush.h | 11 +++++++++++
>> mm/mremap.c | 15 +++++++++++++--
>> 2 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>> index f9f8a3a264f7..c236b66f490b 100644
>> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
>> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>> @@ -80,6 +80,17 @@ static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma,
>> return flush_hugetlb_tlb_pwc_range(vma, start, end, false);
>> }
>>
>> +#define flush_pte_tlb_pwc_range flush_tlb_pwc_range
>> +static inline void flush_pte_tlb_pwc_range(struct vm_area_struct *vma,
>> + unsigned long start, unsigned long end,
>> + bool also_pwc)
>
> This still uses the also_pwc name, which is a bit inconsistent with the
> previous patch.
>
will fix that.
> But, does it even need to be a parameter? AFAICS you always pass true,
> and pwc=true is sort of implied by the name isn't it?
>
I don't have strong opinion about that. I was wondering having flush_pwc
explicitly called out is a better indication of we are flushing page
walk cache. Will drop that in the next update.
-aneesh
^ permalink raw reply
* [PATCH] powerpc/64s: Add FA_DUMP to defconfig
From: Michael Ellerman @ 2021-04-20 4:22 UTC (permalink / raw)
To: linuxppc-dev
FA_DUMP (Firmware Assisted Dump) is a powerpc only feature that should
be enabled in our defconfig to get some build / test coverage.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/configs/ppc64_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 4f05a6652478..72b235ef6f3b 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -50,6 +50,7 @@ CONFIG_PPC_TRANSACTIONAL_MEM=y
CONFIG_KEXEC=y
CONFIG_KEXEC_FILE=y
CONFIG_CRASH_DUMP=y
+CONFIG_FA_DUMP=y
CONFIG_IRQ_ALL_CPUS=y
CONFIG_PPC_64K_PAGES=y
CONFIG_SCHED_SMT=y
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v4 8/9] mm/mremap: Allow arch runtime override
From: Aneesh Kumar K.V @ 2021-04-20 4:30 UTC (permalink / raw)
To: Michael Ellerman, linux-mm, akpm; +Cc: joel, linuxppc-dev, npiggin, kaleshsingh
In-Reply-To: <87bla9d34m.fsf@mpe.ellerman.id.au>
On 4/20/21 9:22 AM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Architectures like ppc64 support faster mremap only with radix
>> translation. Hence allow a runtime check w.r.t support for fast mremap.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/tlb.h | 6 ++++++
>> mm/mremap.c | 15 ++++++++++++++-
>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
>> index 160422a439aa..058918a7cd3c 100644
>> --- a/arch/powerpc/include/asm/tlb.h
>> +++ b/arch/powerpc/include/asm/tlb.h
>> @@ -83,5 +83,11 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
>> }
>> #endif
>>
>> +#define arch_supports_page_tables_move arch_supports_page_tables_move
>> +static inline bool arch_supports_page_tables_move(void)
>> +{
>> + return radix_enabled();
>> +}
>
> Not sure it's worth a respin on its own, but page table*s* move is
> slightly strange phrasing.
>
> arch_supports_move_page_tables() or arch_supports_page_table_move()
> would be more typical.
>
I will switch to arch_supports_page_table_move()
-aneesh
^ permalink raw reply
* [PATCH 1/1] powerpc/pseries/iommu: Fix window size for direct mapping with pmem
From: Leonardo Bras @ 2021-04-20 4:54 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy
Cc: linuxppc-dev, linux-kernel
As of today, if the DDW is big enough to fit (1 << MAX_PHYSMEM_BITS) it's
possible to use direct DMA mapping even with pmem region.
But, if that happens, the window size (len) is set to
(MAX_PHYSMEM_BITS - page_shift) instead of MAX_PHYSMEM_BITS, causing a
pagesize times smaller DDW to be created, being insufficient for correct
usage.
Fix this so the correct window size is used in this case.
Fixes: bf6e2d562bbc4("powerpc/dma: Fallback to dma_ops when persistent memory present")
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/platforms/pseries/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 9fc5217f0c8e..836cbbe0ecc5 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1229,7 +1229,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
if (pmem_present) {
if (query.largest_available_block >=
(1ULL << (MAX_PHYSMEM_BITS - page_shift)))
- len = MAX_PHYSMEM_BITS - page_shift;
+ len = MAX_PHYSMEM_BITS;
else
dev_info(&dev->dev, "Skipping ibm,pmemory");
}
--
2.30.2
^ permalink raw reply related
* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
From: Christophe Leroy @ 2021-04-20 4:55 UTC (permalink / raw)
To: Randy Dunlap, Michael Ellerman, Segher Boessenkool; +Cc: PowerPC, LKML
In-Reply-To: <bd83b06d-ed36-e600-e988-c1e0014fb9cf@infradead.org>
Le 19/04/2021 à 23:39, Randy Dunlap a écrit :
> On 4/19/21 6:16 AM, Michael Ellerman wrote:
>> Randy Dunlap <rdunlap@infradead.org> writes:
>
>>> Sure. I'll post them later today.
>>> They keep FPU and ALTIVEC as independent (build) features.
>>
>> Those patches look OK.
>>
>> But I don't think it makes sense to support that configuration, FPU=n
>> ALTVEC=y. No one is ever going to make a CPU like that. We have enough
>> testing surface due to configuration options, without adding artificial
>> combinations that no one is ever going to use.
>>
>> IMHO :)
>>
>> So I'd rather we just make ALTIVEC depend on FPU.
>
> That's rather simple. See below.
> I'm doing a bunch of randconfig builds with it now.
>
> ---
> From: Randy Dunlap <rdunlap@infradead.org>
> Subject: [PATCH] powerpc: make ALTIVEC depend PPC_FPU
>
> On a kernel config with ALTIVEC=y and PPC_FPU not set/enabled,
> there are build errors:
>
> drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
> enable_kernel_fp();
> ../arch/powerpc/lib/sstep.c: In function 'do_vec_load':
> ../arch/powerpc/lib/sstep.c:637:3: error: implicit declaration of function 'put_vr' [-Werror=implicit-function-declaration]
> 637 | put_vr(rn, &u.v);
> | ^~~~~~
> ../arch/powerpc/lib/sstep.c: In function 'do_vec_store':
> ../arch/powerpc/lib/sstep.c:660:3: error: implicit declaration of function 'get_vr'; did you mean 'get_oc'? [-Werror=implicit-function-declaration]
> 660 | get_vr(rn, &u.v);
> | ^~~~~~
>
> In theory ALTIVEC is independent of PPC_FPU but in practice nobody
> is going to build such a machine, so make ALTIVEC require PPC_FPU
> by depending on PPC_FPU.
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> Cc: lkp@intel.com
> ---
> arch/powerpc/platforms/86xx/Kconfig | 1 +
> arch/powerpc/platforms/Kconfig.cputype | 2 ++
> 2 files changed, 3 insertions(+)
>
> --- linux-next-20210416.orig/arch/powerpc/platforms/86xx/Kconfig
> +++ linux-next-20210416/arch/powerpc/platforms/86xx/Kconfig
> @@ -4,6 +4,7 @@ menuconfig PPC_86xx
> bool "86xx-based boards"
> depends on PPC_BOOK3S_32
> select FSL_SOC
> + select PPC_FPU
> select ALTIVEC
> help
> The Freescale E600 SoCs have 74xx cores.
> --- linux-next-20210416.orig/arch/powerpc/platforms/Kconfig.cputype
> +++ linux-next-20210416/arch/powerpc/platforms/Kconfig.cputype
> @@ -186,6 +186,7 @@ config E300C3_CPU
> config G4_CPU
> bool "G4 (74xx)"
> depends on PPC_BOOK3S_32
> + select PPC_FPU
> select ALTIVEC
>
> endchoice
> @@ -309,6 +310,7 @@ config PHYS_64BIT
>
> config ALTIVEC
> bool "AltiVec Support"
> + depends on PPC_FPU
Shouldn't we do it the other way round ? In extenso make ALTIVEC select PPC_FPU and avoid the two
selects that are above ?
> depends on PPC_BOOK3S_32 || PPC_BOOK3S_64 || (PPC_E500MC && PPC64)
> help
> This option enables kernel support for the Altivec extensions to the
>
^ 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