* Re: [PATCH v2] powerpc: Speed up clear_page by unrolling it
From: Segher Boessenkool @ 2014-10-02 14:17 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev
In-Reply-To: <20141002154421.62073027@kryten>
On Thu, Oct 02, 2014 at 03:44:21PM +1000, Anton Blanchard wrote:
> This assumes cacheline sizes won't grow beyond 512 bytes or
> page sizes wont drop below 1kB,
Or a combination of those.
> Michael found that some versions of gcc produce quite bad code
> (all multiplies), so we give gcc a hand by using shifts and adds.
You can make the code a lot less cluttered as well as making the
generated code independent of compiler version by writing the setup
of twox..eightx in the asm block itself.
Segher
^ permalink raw reply
* Re: [PATCH v2 1/4] pci/msi: Move "no_64bit_msi" flag from powerpc to generic pci_dev
From: Bjorn Helgaas @ 2014-10-02 15:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <1412210052.10667.0.camel@pasglop>
On Thu, Oct 02, 2014 at 10:34:12AM +1000, Benjamin Herrenschmidt wrote:
>
> Some devices have broken 64-bit MSI support which only support some
> address bits (40 to 48 typically). This doesn't work on some platforms
> such as POWER servers, so we need a quirk.
>
> Currently we keep a flag in a powerpc specific data structure which we
> have per PCI device. However this is impractical as we really want the
> driver to set that flag appropriately (and the driver shouldn't touch
> that arch specific data structure).
>
> It's also not unlikely that this limitation will affect other architectures
> in the long run.
>
> So this moves the flag to struct pci_dev instead and adjusts the
> corresponding arch/powerpc code to look for it there. At this point,
> there is no attempt at making other architectures honor it just yet,
> though it appears that x86 doesn't care and always generates 32-bit
> MSI addresses.
I thought you were going to add something in drivers/pci to make this more
generic?
Is it feasible to structure this series as follows?
- add generic stuff (struct pci_dev bit, msi.c checks)
- add quirks to drivers to set the pci_dev bit
- change powerpc to check pci_dev bit instead of pdn bit
- remove powerpc-specific quirks since nobody looks as the pdn bit
I think it would be easier to follow if the powerpc-specific parts weren't
intertwined with the rest.
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
> ---
>
> v2: Rename flag to "no_64bit_msi" as suggested by Bjorn
>
> arch/powerpc/include/asm/pci-bridge.h | 2 --
> arch/powerpc/kernel/pci_64.c | 5 +----
> arch/powerpc/platforms/powernv/pci-ioda.c | 3 +--
> arch/powerpc/platforms/powernv/pci.c | 3 +--
> arch/powerpc/platforms/pseries/msi.c | 2 +-
> include/linux/pci.h | 1 +
> 6 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 4ca90a3..725247b 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -159,8 +159,6 @@ struct pci_dn {
>
> int pci_ext_config_space; /* for pci devices */
>
> - bool force_32bit_msi;
> -
> struct pci_dev *pcidev; /* back-pointer to the pci device */
> #ifdef CONFIG_EEH
> struct eeh_dev *edev; /* eeh device */
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 155013d..d41a831 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
>
> static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> {
> - struct pci_dn *pdn = pci_get_pdn(dev);
> -
> - if (pdn)
> - pdn->force_32bit_msi = true;
> + dev->no_64bit_msi = true;
> }
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index df241b1..a188bb8 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1311,7 +1311,6 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
> unsigned int is_64, struct msi_msg *msg)
> {
> struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev);
> - struct pci_dn *pdn = pci_get_pdn(dev);
> struct irq_data *idata;
> struct irq_chip *ichip;
> unsigned int xive_num = hwirq - phb->msi_base;
> @@ -1327,7 +1326,7 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
> return -ENXIO;
>
> /* Force 32-bit MSI on some broken devices */
> - if (pdn && pdn->force_32bit_msi)
> + if (dev->no_64bit_msi)
> is_64 = 0;
>
> /* Assign XIVE to PE */
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index b854b57..89c6608 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -50,9 +50,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
> {
> struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> struct pnv_phb *phb = hose->private_data;
> - struct pci_dn *pdn = pci_get_pdn(pdev);
>
> - if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
> + if (pdev->no_64bit_msi && !phb->msi32_support)
> return -ENODEV;
>
> return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 18ff462..6fd96d8 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -429,7 +429,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
> */
> again:
> if (type == PCI_CAP_ID_MSI) {
> - if (pdn->force_32bit_msi) {
> + if (pdev->no_64bit_msi) {
> rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
> if (rc < 0) {
> /*
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 96453f9..fc938003 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -331,6 +331,7 @@ struct pci_dev {
> unsigned int is_added:1;
> unsigned int is_busmaster:1; /* device is busmaster */
> unsigned int no_msi:1; /* device may not use msi */
> + unsigned int no_64bit_msi:1; /* Device has broken 64-bit MSIs */
> unsigned int block_cfg_access:1; /* config space access is blocked */
> unsigned int broken_parity_status:1; /* Device generates false positive parity */
> unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
>
>
>
^ permalink raw reply
* Re: [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Bjorn Helgaas @ 2014-10-02 15:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <1412210062.10667.1.camel@pasglop>
On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote:
>
> A number of radeon cards have a HW limitation causing them to be
> unable to generate the full 64-bit of address bits for MSIs. This
> breaks MSIs on some platforms such as POWER machines.
>
> We used to have a powerpc specific quirk to address that on a
> single card, but this doesn't scale very well, this is better
> put under control of the drivers who know precisely what a given
> HW revision can do.
>
> This moves the setting of the quirk flag to the radeon driver
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> v2: This is just adjusted to the new flag name
I'm sorta confused because I got two "v2 2/4" emails a minute or so apart.
I assume they're the same.
>
> arch/powerpc/kernel/pci_64.c | 1 -
> drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index d41a831..5330f6d 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> {
> dev->no_64bit_msi = true;
> }
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too?
> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> index 16807af..e760671 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
> if (rdev->flags & RADEON_IS_AGP)
> return false;
>
> + /*
> + * Older chips have a HW limitation, they can only generate 40 bits
> + * of address for "64-bit" MSIs which breaks on some platforms, notably
> + * IBM POWER servers, so we limit them
> + */
> + if (rdev->family < CHIP_BONAIRE) {
> + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
> + rdev->pdev->no_64bit_msi = true;
> + }
> +
> /* force MSI on */
> if (radeon_msi == 1)
> return true;
>
>
>
^ permalink raw reply
* Re: [PATCH V7 00/17] Enable SRIOV on POWER8
From: Bjorn Helgaas @ 2014-10-02 15:59 UTC (permalink / raw)
To: Wei Yang
Cc: Benjamin Herrenschmidt, linux-pci@vger.kernel.org, Gavin Shan,
Mike Qiu, Guo Chao, linuxppc-dev
In-Reply-To: <20140820033546.GA7669@richard>
On Wed, Aug 20, 2014 at 11:35:46AM +0800, Wei Yang wrote:
> On Tue, Aug 19, 2014 at 10:12:27PM -0500, Bjorn Helgaas wrote:
> >On Tue, Aug 19, 2014 at 9:34 PM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> >> On Tue, Aug 19, 2014 at 03:19:42PM -0600, Bjorn Helgaas wrote:
> >>>On Thu, Jul 24, 2014 at 02:22:10PM +0800, Wei Yang wrote:
> >>>> This patch set enables the SRIOV on POWER8.
> >>>>
> >>>> The gerneral idea is put each VF into one individual PE and allocate required
> >>>> resources like DMA/MSI.
> >>>>
> >>>> One thing special for VF PE is we use M64BT to cover the IOV BAR. M64BT is one
> >>>> hardware on POWER platform to map MMIO address to PE. By using M64BT, we could
> >>>> map one individual VF to a VF PE, which introduce more flexiblity to users.
> >>>>
> >>>> To achieve this effect, we need to do some hack on pci devices's resources.
> >>>> 1. Expand the IOV BAR properly.
> >>>> Done by pnv_pci_ioda_fixup_iov_resources().
> >>>> 2. Shift the IOV BAR properly.
> >>>> Done by pnv_pci_vf_resource_shift().
> >>>> 3. IOV BAR alignment is the total size instead of an individual size on
> >>>> powernv platform.
> >>>> Done by pnv_pcibios_sriov_resource_alignment().
> >>>> 4. Take the IOV BAR alignment into consideration in the sizing and assigning.
> >>>> This is achieved by commit: "PCI: Take additional IOV BAR alignment in
> >>>> sizing and assigning"
> >>>>
> >>>> Test Environment:
> >>>> The SRIOV device tested is Emulex Lancer and Mellanox ConnectX-3 on
> >>>> POWER8.
> >>>>
> >>>> Examples on pass through a VF to guest through vfio:
> >>>> 1. install necessary modules
> >>>> modprobe vfio
> >>>> modprobe vfio-pci
> >>>> 2. retrieve the iommu_group the device belongs to
> >>>> readlink /sys/bus/pci/devices/0000:06:0d.0/iommu_group
> >>>> ../../../../kernel/iommu_groups/26
> >>>> This means it belongs to group 26
> >>>> 3. see how many devices under this iommu_group
> >>>> ls /sys/kernel/iommu_groups/26/devices/
> >>>> 4. unbind the original driver and bind to vfio-pci driver
> >>>> echo 0000:06:0d.0 > /sys/bus/pci/devices/0000:06:0d.0/driver/unbind
> >>>> echo 1102 0002 > /sys/bus/pci/drivers/vfio-pci/new_id
> >>>> Note: this should be done for each device in the same iommu_group
> >>>> 5. Start qemu and pass device through vfio
> >>>> /home/ywywyang/git/qemu-impreza/ppc64-softmmu/qemu-system-ppc64 \
> >>>> -M pseries -m 2048 -enable-kvm -nographic \
> >>>> -drive file=/home/ywywyang/kvm/fc19.img \
> >>>> -monitor telnet:localhost:5435,server,nowait -boot cd \
> >>>> -device "spapr-pci-vfio-host-bridge,id=CXGB3,iommu=26,index=6"
> >>>>
> >>>> Verify this is the exact VF response:
> >>>> 1. ping from a machine in the same subnet(the broadcast domain)
> >>>> 2. run arp -n on this machine
> >>>> 9.115.251.20 ether 00:00:c9:df:ed:bf C eth0
> >>>> 3. ifconfig in the guest
> >>>> # ifconfig eth1
> >>>> eth1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
> >>>> inet 9.115.251.20 netmask 255.255.255.0 broadcast 9.115.251.255
> >>>> inet6 fe80::200:c9ff:fedf:edbf prefixlen 64 scopeid 0x20<link>
> >>>> ether 00:00:c9:df:ed:bf txqueuelen 1000 (Ethernet)
> >>>> RX packets 175 bytes 13278 (12.9 KiB)
> >>>> RX errors 0 dropped 0 overruns 0 frame 0
> >>>> TX packets 58 bytes 9276 (9.0 KiB)
> >>>> TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
> >>>> 4. They have the same MAC address
> >>>>
> >>>> Note: make sure you shutdown other network interfaces in guest.
> >>>>
> >>>> ---
> >>>> v6 -> v7:
> >>>> 1. add IORESOURCE_ARCH flag for IOV BAR on powernv platform.
> >>>> 2. when IOV BAR has IORESOURCE_ARCH flag, the size is retrieved from
> >>>> hardware directly. If not, calculate as usual.
> >>>> 3. reorder the patch set, group them by subsystem:
> >>>> PCI, powerpc, powernv
> >>>> 4. rebase it on 3.16-rc6
> >>>
> >>>This doesn't apply for me on v3.16-rc6:
> >>>
> >>> 02:48:57 ~/linux$ stg rebase v3.16-rc6
> >>> Checking for changes in the working directory ... done
> >>> Rebasing to "v3.16-rc6" ... done
> >>> No patches applied
> >>> 02:49:14 ~/linux$ stg import -M --sign m/wy
> >>> Checking for changes in the working directory ... done
> >>> Importing patch "pci-iov-export-interface-for" ... done
> >>> Importing patch "pci-iov-get-vf-bar-size-from" ... done
> >>> Importing patch "pci-add-weak" ... done
> >>> Importing patch "pci-take-additional-iov-bar" ... done
> >>> Importing patch "powerpc-pci-don-t-unset-pci" ... done
> >>> Importing patch "powerpc-pci-define" ... done
> >>> Importing patch "powrepc-pci-refactor-pci_dn" ... done
> >>> Importing patch "powerpc-powernv-use-pci_dn-in" ... error: patch failed:
> >>> arch/powerpc/platforms/powernv/pci.c:376
> >>> error: arch/powerpc/platforms/powernv/pci.c: patch does not apply
> >>> stg import: Diff does not apply cleanly
> >>>
> >>>What am I missing?
> >>>
> >>>I assume you intend these all to go through my tree just to keep them all
> >>>together. The ideal rebase target for me would be v3.17-rc1.
> >>
> >> Ok, I will rebase it on v3.17-rc1 upstream. While I guess the conflict is due
> >> to some patches from Gavin, which is not merged at that moment. I will make
> >> sure it applies to v3.17-rc1.
> >
> >I tried applying them on v3.16-rc6 as well as on every change to
> >arch/powerpc/platforms/powernv/pci.c between v3.16-rc6 and v3.17-rc1,
> >and none applied cleanly. Patches you post should be based on some
> >upstream tag, not on something that includes unmerged patches.
>
> Sorry about this, I will pay attention to this next time.
I haven't seen any more on this series, and I'm assuming you'll post a
rebased series (maybe you're waiting for v3.18-rc1?). I'm just checking to
make sure you're not waiting for something from me...
Bjorn
^ permalink raw reply
* [PATCH] powerpc: fix sys_call_table declaration
From: Romeo Cane @ 2014-10-02 14:41 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr
Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com>
---
arch/powerpc/include/asm/syscall.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index b54b2ad..528ba9d 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -17,7 +17,7 @@
/* ftrace syscalls requires exporting the sys_call_table */
#ifdef CONFIG_FTRACE_SYSCALLS
-extern const unsigned long *sys_call_table;
+extern const unsigned long sys_call_table[];
#endif /* CONFIG_FTRACE_SYSCALLS */
static inline long syscall_get_nr(struct task_struct *task,
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH v2 2/3] powerpc/kvm/book3s_hv: Enable CPUs to run guest after waking up from fast-sleep
From: Shreyas B Prabhu @ 2014-10-02 16:39 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, Rafael J. Wysocki, Paul Mackerras, Preeti U Murthy,
linuxppc-dev
In-Reply-To: <1412149560-2953-3-git-send-email-shreyas@linux.vnet.ibm.com>
CCing Rafael J. Wysocki and linux-pm@vger.kernel.org
On Wednesday 01 October 2014 01:15 PM, Shreyas B. Prabhu wrote:
> When guests have to be launched, the secondary threads which are offline
> are woken up to run the guests. Today these threads wake up from nap
> and check if they have to run guests. Now that the offline secondary
> threads can go to fastsleep or going ahead a deeper idle state such as winkle,
> add this check in the wakeup from any of the deep idle states path as well.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Suggested-by: "Srivatsa S. Bhat" <srivatsa@mit.edu>
> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
> [ Changelog added by <preeti@linux.vnet.ibm.com> ]
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/exceptions-64s.S | 35 ++++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 050f79a..c64f3cc0 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -100,25 +100,8 @@ system_reset_pSeries:
> SET_SCRATCH0(r13)
> #ifdef CONFIG_PPC_P7_NAP
> BEGIN_FTR_SECTION
> - /* Running native on arch 2.06 or later, check if we are
> - * waking up from nap. We only handle no state loss and
> - * supervisor state loss. We do -not- handle hypervisor
> - * state loss at this time.
> - */
> - mfspr r13,SPRN_SRR1
> - rlwinm. r13,r13,47-31,30,31
> - beq 9f
>
> - /* waking up from powersave (nap) state */
> - cmpwi cr1,r13,2
> - /* Total loss of HV state is fatal, we could try to use the
> - * PIR to locate a PACA, then use an emergency stack etc...
> - * OPAL v3 based powernv platforms have new idle states
> - * which fall in this catagory.
> - */
> - bgt cr1,8f
> GET_PACA(r13)
> -
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> li r0,KVM_HWTHREAD_IN_KERNEL
> stb r0,HSTATE_HWTHREAD_STATE(r13)
> @@ -131,13 +114,27 @@ BEGIN_FTR_SECTION
> 1:
> #endif
>
> + /* Running native on arch 2.06 or later, check if we are
> + * waking up from nap. We only handle no state loss and
> + * supervisor state loss. We do -not- handle hypervisor
> + * state loss at this time.
> + */
> + mfspr r13,SPRN_SRR1
> + rlwinm. r13,r13,47-31,30,31
> + beq 9f
> +
> + /* waking up from powersave (nap) state */
> + cmpwi cr1,r13,2
> + GET_PACA(r13)
> +
> + bgt cr1,8f
> +
> beq cr1,2f
> b power7_wakeup_noloss
> 2: b power7_wakeup_loss
>
> /* Fast Sleep wakeup on PowerNV */
> -8: GET_PACA(r13)
> - b power7_wakeup_tb_loss
> +8: b power7_wakeup_tb_loss
>
> 9:
> END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
>
^ permalink raw reply
* Re: [PATCH v2 0/3] powernv/cpuidle: Fastsleep workaround and fixes
From: Shreyas B Prabhu @ 2014-10-02 16:40 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Srivatsa S. Bhat, linux-pm, linux-kernel, Paul Mackerras,
Preeti U. Murthy, linuxppc-dev
In-Reply-To: <2978842.I7Hdqv7lNe@vostro.rjw.lan>
On Thursday 02 October 2014 02:16 AM, Rafael J. Wysocki wrote:
> On Wednesday, October 01, 2014 01:15:57 PM Shreyas B. Prabhu wrote:
>> Fast sleep is an idle state, where the core and the L1 and L2
>> caches are brought down to a threshold voltage. This also means that
>> the communication between L2 and L3 caches have to be fenced. However
>> the current P8 chips have a bug wherein this fencing between L2 and
>> L3 caches get delayed by a cpu cycle. This can delay L3 response to
>> the other cpus if they request for data during this time. Thus they
>> would fetch the same data from the memory which could lead to data
>> corruption if L3 cache is not flushed.
>>
>> This series overcomes above problem in kernel.
>>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: linux-pm@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: Srivatsa S. Bhat <srivatsa@mit.edu>
>> Cc: Preeti U. Murthy <preeti@linux.vnet.ibm.com>
>> Cc: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
>>
>> v2:
>> Rebased on 3.17-rc7
>> Split from 'powerpc/powernv: Support for fastsleep and winkle'
>>
>> v1:
>> https://lkml.org/lkml/2014/8/25/446
>>
>> Preeti U Murthy (1):
>> powerpc/powernv/cpuidle: Add workaround to enable fastsleep
>>
>> Shreyas B. Prabhu (1):
>> powerpc/kvm/book3s_hv: Enable CPUs to run guest after waking up from
>> fast-sleep
>>
>> Srivatsa S. Bhat (1):
>> powerpc/powernv: Enable Offline CPUs to enter deep idle states
>>
>> arch/powerpc/include/asm/machdep.h | 3 +
>> arch/powerpc/include/asm/opal.h | 7 ++
>> arch/powerpc/include/asm/processor.h | 4 +-
>> arch/powerpc/kernel/exceptions-64s.S | 35 ++++----
>> arch/powerpc/kernel/idle.c | 19 ++++
>> arch/powerpc/kernel/idle_power7.S | 2 +-
>> arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
>> arch/powerpc/platforms/powernv/powernv.h | 7 ++
>> arch/powerpc/platforms/powernv/setup.c | 118 +++++++++++++++++++++++++
>> arch/powerpc/platforms/powernv/smp.c | 11 ++-
>> drivers/cpuidle/cpuidle-powernv.c | 13 ++-
>> 11 files changed, 194 insertions(+), 26 deletions(-)
>
> [2/3] seems to be missig from the series.
>
> Also, since that mostly modifies arch/powerpc, I think it should go through
> that tree. I'm fine with the cpuidle-powernv changes in [1/3] and [3/3].
>
Hi Rafael,
Thanks for looking into this. The second patch is an independent fix in the
powerpc exception handler. To be safe I am ccing you and linux-pm list on that
patch now.
Thanks,
Shreyas
^ permalink raw reply
* Re: [PATCH v2 1/4] pci/msi: Move "no_64bit_msi" flag from powerpc to generic pci_dev
From: Benjamin Herrenschmidt @ 2014-10-02 21:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <20141002153101.GB18056@google.com>
On Thu, 2014-10-02 at 09:31 -0600, Bjorn Helgaas wrote:
> > So this moves the flag to struct pci_dev instead and adjusts the
> > corresponding arch/powerpc code to look for it there. At this point,
> > there is no attempt at making other architectures honor it just yet,
> > though it appears that x86 doesn't care and always generates 32-bit
> > MSI addresses.
>
> I thought you were going to add something in drivers/pci to make this more
> generic?
I don't see how. The only thing I can add is the check which I still
plan to do. But the decision on the address is in the arch.
> Is it feasible to structure this series as follows?
>
> - add generic stuff (struct pci_dev bit, msi.c checks)
> - add quirks to drivers to set the pci_dev bit
> - change powerpc to check pci_dev bit instead of pdn bit
> - remove powerpc-specific quirks since nobody looks as the pdn bit
>
> I think it would be easier to follow if the powerpc-specific parts weren't
> intertwined with the rest.
Well, in that case the first patch just adds one bit to pci_dev and
maybe one check for address to msi.c. Unfortunately there isn't more
I can do generically. But ok, I'll do it that way.
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: <stable@vger.kernel.org>
> > ---
> >
> > v2: Rename flag to "no_64bit_msi" as suggested by Bjorn
> >
> > arch/powerpc/include/asm/pci-bridge.h | 2 --
> > arch/powerpc/kernel/pci_64.c | 5 +----
> > arch/powerpc/platforms/powernv/pci-ioda.c | 3 +--
> > arch/powerpc/platforms/powernv/pci.c | 3 +--
> > arch/powerpc/platforms/pseries/msi.c | 2 +-
> > include/linux/pci.h | 1 +
> > 6 files changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> > index 4ca90a3..725247b 100644
> > --- a/arch/powerpc/include/asm/pci-bridge.h
> > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > @@ -159,8 +159,6 @@ struct pci_dn {
> >
> > int pci_ext_config_space; /* for pci devices */
> >
> > - bool force_32bit_msi;
> > -
> > struct pci_dev *pcidev; /* back-pointer to the pci device */
> > #ifdef CONFIG_EEH
> > struct eeh_dev *edev; /* eeh device */
> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> > index 155013d..d41a831 100644
> > --- a/arch/powerpc/kernel/pci_64.c
> > +++ b/arch/powerpc/kernel/pci_64.c
> > @@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
> >
> > static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> > {
> > - struct pci_dn *pdn = pci_get_pdn(dev);
> > -
> > - if (pdn)
> > - pdn->force_32bit_msi = true;
> > + dev->no_64bit_msi = true;
> > }
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> > index df241b1..a188bb8 100644
> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > @@ -1311,7 +1311,6 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
> > unsigned int is_64, struct msi_msg *msg)
> > {
> > struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev);
> > - struct pci_dn *pdn = pci_get_pdn(dev);
> > struct irq_data *idata;
> > struct irq_chip *ichip;
> > unsigned int xive_num = hwirq - phb->msi_base;
> > @@ -1327,7 +1326,7 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
> > return -ENXIO;
> >
> > /* Force 32-bit MSI on some broken devices */
> > - if (pdn && pdn->force_32bit_msi)
> > + if (dev->no_64bit_msi)
> > is_64 = 0;
> >
> > /* Assign XIVE to PE */
> > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> > index b854b57..89c6608 100644
> > --- a/arch/powerpc/platforms/powernv/pci.c
> > +++ b/arch/powerpc/platforms/powernv/pci.c
> > @@ -50,9 +50,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
> > {
> > struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > struct pnv_phb *phb = hose->private_data;
> > - struct pci_dn *pdn = pci_get_pdn(pdev);
> >
> > - if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
> > + if (pdev->no_64bit_msi && !phb->msi32_support)
> > return -ENODEV;
> >
> > return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
> > diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> > index 18ff462..6fd96d8 100644
> > --- a/arch/powerpc/platforms/pseries/msi.c
> > +++ b/arch/powerpc/platforms/pseries/msi.c
> > @@ -429,7 +429,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
> > */
> > again:
> > if (type == PCI_CAP_ID_MSI) {
> > - if (pdn->force_32bit_msi) {
> > + if (pdev->no_64bit_msi) {
> > rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
> > if (rc < 0) {
> > /*
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 96453f9..fc938003 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -331,6 +331,7 @@ struct pci_dev {
> > unsigned int is_added:1;
> > unsigned int is_busmaster:1; /* device is busmaster */
> > unsigned int no_msi:1; /* device may not use msi */
> > + unsigned int no_64bit_msi:1; /* Device has broken 64-bit MSIs */
> > unsigned int block_cfg_access:1; /* config space access is blocked */
> > unsigned int broken_parity_status:1; /* Device generates false positive parity */
> > unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
> >
> >
> >
^ permalink raw reply
* Re: [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Benjamin Herrenschmidt @ 2014-10-02 21:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci, Anton Blanchard, Brian King,
Yijing Wang, Takashi Iwai, Alex Deucher
In-Reply-To: <20141002153402.GC18056@google.com>
On Thu, 2014-10-02 at 09:34 -0600, Bjorn Helgaas wrote:
> On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote:
> >
> > A number of radeon cards have a HW limitation causing them to be
> > unable to generate the full 64-bit of address bits for MSIs. This
> > breaks MSIs on some platforms such as POWER machines.
> >
> > We used to have a powerpc specific quirk to address that on a
> > single card, but this doesn't scale very well, this is better
> > put under control of the drivers who know precisely what a given
> > HW revision can do.
> >
> > This moves the setting of the quirk flag to the radeon driver
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >
> > v2: This is just adjusted to the new flag name
>
> I'm sorta confused because I got two "v2 2/4" emails a minute or so apart.
> I assume they're the same.
Yes, evo blew up while sending the series the first time around :(
> >
> > arch/powerpc/kernel/pci_64.c | 1 -
> > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++
> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> > index d41a831..5330f6d 100644
> > --- a/arch/powerpc/kernel/pci_64.c
> > +++ b/arch/powerpc/kernel/pci_64.c
> > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> > {
> > dev->no_64bit_msi = true;
> > }
> > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
>
> Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too?
>
> > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> > index 16807af..e760671 100644
> > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
> > if (rdev->flags & RADEON_IS_AGP)
> > return false;
> >
> > + /*
> > + * Older chips have a HW limitation, they can only generate 40 bits
> > + * of address for "64-bit" MSIs which breaks on some platforms, notably
> > + * IBM POWER servers, so we limit them
> > + */
> > + if (rdev->family < CHIP_BONAIRE) {
> > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
> > + rdev->pdev->no_64bit_msi = true;
> > + }
> > +
> > /* force MSI on */
> > if (radeon_msi == 1)
> > return true;
> >
> >
> >
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] powerpc: Fix warning reported by verify_cpu_node_mapping()
From: Nishanth Aravamudan @ 2014-10-02 21:13 UTC (permalink / raw)
To: Li Zhong; +Cc: linuxppc-dev, Nathan Fontenot, paulus
In-Reply-To: <1409132041-11890-1-git-send-email-zhong@linux.vnet.ibm.com>
Ben & Michael,
What's the status of these patches?
Thanks,
Nish
On 27.08.2014 [17:33:59 +0800], Li Zhong wrote:
> With commit 2fabf084b6ad ("powerpc: reorder per-cpu NUMA information's
> initialization"), during boottime, cpu_numa_callback() is called
> earlier(before their online) for each cpu, and verify_cpu_node_mapping()
> uses cpu_to_node() to check whether siblings are in the same node.
>
> It skips the checking for siblings that are not online yet. So the only
> check done here is for the bootcpu, which is online at that time. But
> the per-cpu numa_node cpu_to_node() uses hasn't been set up yet (which
> will be set up in smp_prepare_cpus()).
>
> So I saw something like following reported:
> [ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the same
> node!
>
> As we don't actually do the checking during this early stage, so maybe
> we could directly call numa_setup_cpu() in do_init_bootmem().
>
> Cc: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index d7737a5..9918c02 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)
> * early in boot, cf. smp_prepare_cpus().
> */
> for_each_possible_cpu(cpu) {
> - cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
> - (void *)(unsigned long)cpu);
> + numa_setup_cpu((unsigned long)cpu);
> }
> }
>
> --
> 1.9.1
>
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] powerpc: Fix warning reported by verify_cpu_node_mapping()
From: Benjamin Herrenschmidt @ 2014-10-02 21:28 UTC (permalink / raw)
To: Nishanth Aravamudan; +Cc: paulus, linuxppc-dev, Nathan Fontenot, Li Zhong
In-Reply-To: <20141002211305.GB12862@linux.vnet.ibm.com>
On Thu, 2014-10-02 at 14:13 -0700, Nishanth Aravamudan wrote:
> Ben & Michael,
>
> What's the status of these patches?
Waiting for somebody to review them ? :-)
Cheers,
Ben.
> Thanks,
> Nish
>
> On 27.08.2014 [17:33:59 +0800], Li Zhong wrote:
> > With commit 2fabf084b6ad ("powerpc: reorder per-cpu NUMA information's
> > initialization"), during boottime, cpu_numa_callback() is called
> > earlier(before their online) for each cpu, and verify_cpu_node_mapping()
> > uses cpu_to_node() to check whether siblings are in the same node.
> >
> > It skips the checking for siblings that are not online yet. So the only
> > check done here is for the bootcpu, which is online at that time. But
> > the per-cpu numa_node cpu_to_node() uses hasn't been set up yet (which
> > will be set up in smp_prepare_cpus()).
> >
> > So I saw something like following reported:
> > [ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the same
> > node!
> >
> > As we don't actually do the checking during this early stage, so maybe
> > we could directly call numa_setup_cpu() in do_init_bootmem().
> >
> > Cc: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> > Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/mm/numa.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index d7737a5..9918c02 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -1128,8 +1128,7 @@ void __init do_init_bootmem(void)
> > * early in boot, cf. smp_prepare_cpus().
> > */
> > for_each_possible_cpu(cpu) {
> > - cpu_numa_callback(&ppc64_numa_nb, CPU_UP_PREPARE,
> > - (void *)(unsigned long)cpu);
> > + numa_setup_cpu((unsigned long)cpu);
> > }
> > }
> >
> > --
> > 1.9.1
> >
^ permalink raw reply
* Re: [PATCH] powerpc: fix sys_call_table declaration
From: Benjamin Herrenschmidt @ 2014-10-02 21:34 UTC (permalink / raw)
To: Romeo Cane; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20141002144131.GA3855@rcane-VirtualBox>
On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote:
> Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr
Care to elaborate ?
Ben.
> Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com>
> ---
> arch/powerpc/include/asm/syscall.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index b54b2ad..528ba9d 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -17,7 +17,7 @@
>
> /* ftrace syscalls requires exporting the sys_call_table */
> #ifdef CONFIG_FTRACE_SYSCALLS
> -extern const unsigned long *sys_call_table;
> +extern const unsigned long sys_call_table[];
> #endif /* CONFIG_FTRACE_SYSCALLS */
>
> static inline long syscall_get_nr(struct task_struct *task,
^ permalink raw reply
* Re: [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Bjorn Helgaas @ 2014-10-02 21:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci@vger.kernel.org,
Anton Blanchard, Brian King, Yijing Wang, Takashi Iwai,
Alex Deucher
In-Reply-To: <1412283745.28143.23.camel@pasglop>
On Thu, Oct 2, 2014 at 3:02 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2014-10-02 at 09:34 -0600, Bjorn Helgaas wrote:
>> On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote:
>> >
>> > A number of radeon cards have a HW limitation causing them to be
>> > unable to generate the full 64-bit of address bits for MSIs. This
>> > breaks MSIs on some platforms such as POWER machines.
>> >
>> > We used to have a powerpc specific quirk to address that on a
>> > single card, but this doesn't scale very well, this is better
>> > put under control of the drivers who know precisely what a given
>> > HW revision can do.
>> >
>> > This moves the setting of the quirk flag to the radeon driver
>> >
>> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> > ---
>> >
>> > v2: This is just adjusted to the new flag name
>>
>> I'm sorta confused because I got two "v2 2/4" emails a minute or so apart.
>> I assume they're the same.
>
> Yes, evo blew up while sending the series the first time around :(
No problem. My real question is below, but you probably missed it
because of my email gripe :)
>> > arch/powerpc/kernel/pci_64.c | 1 -
>> > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++
>> > 2 files changed, 10 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
>> > index d41a831..5330f6d 100644
>> > --- a/arch/powerpc/kernel/pci_64.c
>> > +++ b/arch/powerpc/kernel/pci_64.c
>> > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev)
>> > {
>> > dev->no_64bit_msi = true;
>> > }
>> > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
>> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
>>
>> Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too?
>>
>> > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> > index 16807af..e760671 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
>> > if (rdev->flags & RADEON_IS_AGP)
>> > return false;
>> >
>> > + /*
>> > + * Older chips have a HW limitation, they can only generate 40 bits
>> > + * of address for "64-bit" MSIs which breaks on some platforms, notably
>> > + * IBM POWER servers, so we limit them
>> > + */
>> > + if (rdev->family < CHIP_BONAIRE) {
>> > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
>> > + rdev->pdev->no_64bit_msi = true;
>> > + }
>> > +
>> > /* force MSI on */
>> > if (radeon_msi == 1)
>> > return true;
>> >
>> >
>> >
>
>
^ permalink raw reply
* Re: [PATCH v2 1/4] pci/msi: Move "no_64bit_msi" flag from powerpc to generic pci_dev
From: Bjorn Helgaas @ 2014-10-02 21:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Dave Airlie, linux-pci@vger.kernel.org,
Anton Blanchard, Brian King, Yijing Wang, Takashi Iwai,
Alex Deucher
In-Reply-To: <1412283716.28143.22.camel@pasglop>
On Thu, Oct 2, 2014 at 3:01 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2014-10-02 at 09:31 -0600, Bjorn Helgaas wrote:
>
>> > So this moves the flag to struct pci_dev instead and adjusts the
>> > corresponding arch/powerpc code to look for it there. At this point,
>> > there is no attempt at making other architectures honor it just yet,
>> > though it appears that x86 doesn't care and always generates 32-bit
>> > MSI addresses.
>>
>> I thought you were going to add something in drivers/pci to make this more
>> generic?
>
> I don't see how. The only thing I can add is the check which I still
> plan to do. But the decision on the address is in the arch.
Yep, the check is what I meant. That should at least allow the driver
to fail gracefully instead of crashing the machine.
>> Is it feasible to structure this series as follows?
>>
>> - add generic stuff (struct pci_dev bit, msi.c checks)
>> - add quirks to drivers to set the pci_dev bit
>> - change powerpc to check pci_dev bit instead of pdn bit
>> - remove powerpc-specific quirks since nobody looks as the pdn bit
>>
>> I think it would be easier to follow if the powerpc-specific parts weren't
>> intertwined with the rest.
>
> Well, in that case the first patch just adds one bit to pci_dev and
> maybe one check for address to msi.c. Unfortunately there isn't more
> I can do generically. But ok, I'll do it that way.
>
>> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> > CC: <stable@vger.kernel.org>
>> > ---
>> >
>> > v2: Rename flag to "no_64bit_msi" as suggested by Bjorn
>> >
>> > arch/powerpc/include/asm/pci-bridge.h | 2 --
>> > arch/powerpc/kernel/pci_64.c | 5 +----
>> > arch/powerpc/platforms/powernv/pci-ioda.c | 3 +--
>> > arch/powerpc/platforms/powernv/pci.c | 3 +--
>> > arch/powerpc/platforms/pseries/msi.c | 2 +-
>> > include/linux/pci.h | 1 +
>> > 6 files changed, 5 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>> > index 4ca90a3..725247b 100644
>> > --- a/arch/powerpc/include/asm/pci-bridge.h
>> > +++ b/arch/powerpc/include/asm/pci-bridge.h
>> > @@ -159,8 +159,6 @@ struct pci_dn {
>> >
>> > int pci_ext_config_space; /* for pci devices */
>> >
>> > - bool force_32bit_msi;
>> > -
>> > struct pci_dev *pcidev; /* back-pointer to the pci device */
>> > #ifdef CONFIG_EEH
>> > struct eeh_dev *edev; /* eeh device */
>> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
>> > index 155013d..d41a831 100644
>> > --- a/arch/powerpc/kernel/pci_64.c
>> > +++ b/arch/powerpc/kernel/pci_64.c
>> > @@ -269,10 +269,7 @@ EXPORT_SYMBOL(pcibus_to_node);
>> >
>> > static void quirk_radeon_32bit_msi(struct pci_dev *dev)
>> > {
>> > - struct pci_dn *pdn = pci_get_pdn(dev);
>> > -
>> > - if (pdn)
>> > - pdn->force_32bit_msi = true;
>> > + dev->no_64bit_msi = true;
>> > }
>> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
>> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
>> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> > index df241b1..a188bb8 100644
>> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> > @@ -1311,7 +1311,6 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
>> > unsigned int is_64, struct msi_msg *msg)
>> > {
>> > struct pnv_ioda_pe *pe = pnv_ioda_get_pe(dev);
>> > - struct pci_dn *pdn = pci_get_pdn(dev);
>> > struct irq_data *idata;
>> > struct irq_chip *ichip;
>> > unsigned int xive_num = hwirq - phb->msi_base;
>> > @@ -1327,7 +1326,7 @@ static int pnv_pci_ioda_msi_setup(struct pnv_phb *phb, struct pci_dev *dev,
>> > return -ENXIO;
>> >
>> > /* Force 32-bit MSI on some broken devices */
>> > - if (pdn && pdn->force_32bit_msi)
>> > + if (dev->no_64bit_msi)
>> > is_64 = 0;
>> >
>> > /* Assign XIVE to PE */
>> > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> > index b854b57..89c6608 100644
>> > --- a/arch/powerpc/platforms/powernv/pci.c
>> > +++ b/arch/powerpc/platforms/powernv/pci.c
>> > @@ -50,9 +50,8 @@ static int pnv_msi_check_device(struct pci_dev* pdev, int nvec, int type)
>> > {
>> > struct pci_controller *hose = pci_bus_to_host(pdev->bus);
>> > struct pnv_phb *phb = hose->private_data;
>> > - struct pci_dn *pdn = pci_get_pdn(pdev);
>> >
>> > - if (pdn && pdn->force_32bit_msi && !phb->msi32_support)
>> > + if (pdev->no_64bit_msi && !phb->msi32_support)
>> > return -ENODEV;
>> >
>> > return (phb && phb->msi_bmp.bitmap) ? 0 : -ENODEV;
>> > diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
>> > index 18ff462..6fd96d8 100644
>> > --- a/arch/powerpc/platforms/pseries/msi.c
>> > +++ b/arch/powerpc/platforms/pseries/msi.c
>> > @@ -429,7 +429,7 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
>> > */
>> > again:
>> > if (type == PCI_CAP_ID_MSI) {
>> > - if (pdn->force_32bit_msi) {
>> > + if (pdev->no_64bit_msi) {
>> > rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN, nvec);
>> > if (rc < 0) {
>> > /*
>> > diff --git a/include/linux/pci.h b/include/linux/pci.h
>> > index 96453f9..fc938003 100644
>> > --- a/include/linux/pci.h
>> > +++ b/include/linux/pci.h
>> > @@ -331,6 +331,7 @@ struct pci_dev {
>> > unsigned int is_added:1;
>> > unsigned int is_busmaster:1; /* device is busmaster */
>> > unsigned int no_msi:1; /* device may not use msi */
>> > + unsigned int no_64bit_msi:1; /* Device has broken 64-bit MSIs */
>> > unsigned int block_cfg_access:1; /* config space access is blocked */
>> > unsigned int broken_parity_status:1; /* Device generates false positive parity */
>> > unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting variant */
>> >
>> >
>> >
>
>
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] powerpc: Fix warning reported by verify_cpu_node_mapping()
From: Nishanth Aravamudan @ 2014-10-02 21:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: paulus, linuxppc-dev, Nathan Fontenot, Li Zhong
In-Reply-To: <1412285299.28143.33.camel@pasglop>
On 03.10.2014 [07:28:19 +1000], Benjamin Herrenschmidt wrote:
> On Thu, 2014-10-02 at 14:13 -0700, Nishanth Aravamudan wrote:
> > Ben & Michael,
> >
> > What's the status of these patches?
>
> Waiting for somebody to review them ? :-)
Heh, I acked them about a month ago, I can give a stronger Reviewed-by,
if you prefer?
-Nish
^ permalink raw reply
* [PATCH] [RFC] powerpc/vphn: fix endian issue in NUMA device node code
From: Greg Kurz @ 2014-10-02 21:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nishanth Aravamudan
The associativity domain numbers are obtained from the hypervisor through
registers and written into memory by the guest: the packed array passed to
vphn_unpack_associativity() is then native-endian, unlike what was assumed
in the following commit:
commit b08a2a12e44eaec5024b2b969f4fcb98169d1ca3
Author: Alistair Popple <alistair@popple.id.au>
Date: Wed Aug 7 02:01:44 2013 +1000
powerpc: Make NUMA device node code endian safe
If a CPU home node changes, the topology gets filled with
bogus values. This leads to severe performance breakdowns.
This patch does two things:
- extract values from the packed array with shifts, in order to be endian
neutral
- convert the resulting values to be32 as expected
Suggested-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
I could test this code in a userland program and obtain the same
result in little and big endian. If the 64-bit packed values
are:
0x8001ffffffff0000
0x112200003344ffff
0xffff000055660000
0x7788000099aaffff
0xffff8002ffffffff
0xffffffffffffffff
then the unpacked array is:
0x00000007
0x00000001
0xffffffff
0x00001122
0x00003344
0xffffffff
0x00005566
0x00007788
0x000099aa
0xffffffff
0x00000002
0xffffffff
0xffffffff
I could not test in a PowerVM guest though, hence the RFC.
--
Greg
arch/powerpc/mm/numa.c | 50 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b835bf0..06af179 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1369,38 +1369,68 @@ static int update_cpu_associativity_changes_mask(void)
#define VPHN_ASSOC_BUFSIZE (6*sizeof(u64)/sizeof(u32) + 1)
/*
+ * The associativity values are either 16-bit (VPHN_FIELD_MSB) or 32-bit (data
+ * or VPHN_FIELD_UNUSED). We hence need to parse the packed array into 16-bit
+ * chunks. Let's do that with bit shifts to be endian neutral.
+ *
+ * --- 16-bit chunks -->
+ * _________________________
+ * | 0 | 1 | 2 | 3 | packed[0]
+ * -------------------------
+ * _________________________
+ * | 4 | 5 | 6 | 7 | packed[1]
+ * -------------------------
+ * ...
+ * _________________________
+ * | 20 | 21 | 22 | 23 | packed[5]
+ * -------------------------
+ *
+ * >>48 >>32 >>16 >>0 <-- needed bit shift
+ *
+ * We only care for the 2 lower bits of the chunk index to compute the shift.
+ */
+static inline u16 read_vphn_chunk(const long *packed, unsigned int i)
+{
+ return packed[i >> 2] >> ((~i & 3) << 4);
+}
+
+/*
* Convert the associativity domain numbers returned from the hypervisor
* to the sequence they would appear in the ibm,associativity property.
*/
static int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
{
- int i, nr_assoc_doms = 0;
+ unsigned int i, j, nr_assoc_doms = 0;
const __be16 *field = (const __be16 *) packed;
#define VPHN_FIELD_UNUSED (0xffff)
#define VPHN_FIELD_MSB (0x8000)
#define VPHN_FIELD_MASK (~VPHN_FIELD_MSB)
- for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
- if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
+ for (i = 1, j = 0; i < VPHN_ASSOC_BUFSIZE; i++) {
+ u16 field = read_vphn_chunk(packed, j);
+
+ if (field == VPHN_FIELD_UNUSED) {
/* All significant fields processed, and remaining
* fields contain the reserved value of all 1's.
* Just store them.
*/
- unpacked[i] = *((__be32 *)field);
- field += 2;
+ unpacked[i] = (VPHN_FIELD_UNUSED << 16 |
+ VPHN_FIELD_UNUSED);
+ j += 2;
} else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
/* Data is in the lower 15 bits of this field */
- unpacked[i] = cpu_to_be32(
- be16_to_cpup(field) & VPHN_FIELD_MASK);
- field++;
+ unpacked[i] = cpu_to_be32(field & VPHN_FIELD_MASK);
+ j++;
nr_assoc_doms++;
} else {
/* Data is in the lower 15 bits of this field
* concatenated with the next 16 bit field
*/
- unpacked[i] = *((__be32 *)field);
- field += 2;
+ unpacked[i] =
+ cpu_to_be32((u32) field << 16 |
+ read_vphn_chunk(packed, j + 1));
+ j += 2;
nr_assoc_doms++;
}
}
^ permalink raw reply related
* Re: [PATCH v2 2/4] gpu/radeon: Move 64-bit MSI quirk from arch to driver
From: Benjamin Herrenschmidt @ 2014-10-02 22:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linuxppc-dev, Dave Airlie, linux-pci@vger.kernel.org,
Anton Blanchard, Brian King, Yijing Wang, Takashi Iwai,
Alex Deucher
In-Reply-To: <CAErSpo4XYYJGZvAH7bQp6DXdBtLFT4e8WR6ve3mdV1LY0JdgPA@mail.gmail.com>
On Thu, 2014-10-02 at 15:44 -0600, Bjorn Helgaas wrote:
> On Thu, Oct 2, 2014 at 3:02 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Thu, 2014-10-02 at 09:34 -0600, Bjorn Helgaas wrote:
> >> On Thu, Oct 02, 2014 at 10:34:22AM +1000, Benjamin Herrenschmidt wrote:
> >> >
> >> > A number of radeon cards have a HW limitation causing them to be
> >> > unable to generate the full 64-bit of address bits for MSIs. This
> >> > breaks MSIs on some platforms such as POWER machines.
> >> >
> >> > We used to have a powerpc specific quirk to address that on a
> >> > single card, but this doesn't scale very well, this is better
> >> > put under control of the drivers who know precisely what a given
> >> > HW revision can do.
> >> >
> >> > This moves the setting of the quirk flag to the radeon driver
> >> >
> >> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> > ---
> >> >
> >> > v2: This is just adjusted to the new flag name
> >>
> >> I'm sorta confused because I got two "v2 2/4" emails a minute or so apart.
> >> I assume they're the same.
> >
> > Yes, evo blew up while sending the series the first time around :(
>
> No problem. My real question is below, but you probably missed it
> because of my email gripe :)
Heh ok :-)
> >> > arch/powerpc/kernel/pci_64.c | 1 -
> >> > drivers/gpu/drm/radeon/radeon_irq_kms.c | 10 ++++++++++
> >> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> >> > index d41a831..5330f6d 100644
> >> > --- a/arch/powerpc/kernel/pci_64.c
> >> > +++ b/arch/powerpc/kernel/pci_64.c
> >> > @@ -271,5 +271,4 @@ static void quirk_radeon_32bit_msi(struct pci_dev *dev)
> >> > {
> >> > dev->no_64bit_msi = true;
> >> > }
> >> > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x68f2, quirk_radeon_32bit_msi);
> >> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0xaa68, quirk_radeon_32bit_msi);
> >>
> >> Why do we keep the 0xaa68 quirk? Shouldn't that be made generic, too?
aa68 is the audio part, it's removed by the audio driver patch.
But I can break things down into smaller bits as you suggested. I'll try
to get that sorted later today.
Cheers,
Ben.
> >> > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> >> > index 16807af..e760671 100644
> >> > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> >> > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> >> > @@ -202,6 +202,16 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
> >> > if (rdev->flags & RADEON_IS_AGP)
> >> > return false;
> >> >
> >> > + /*
> >> > + * Older chips have a HW limitation, they can only generate 40 bits
> >> > + * of address for "64-bit" MSIs which breaks on some platforms, notably
> >> > + * IBM POWER servers, so we limit them
> >> > + */
> >> > + if (rdev->family < CHIP_BONAIRE) {
> >> > + dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
> >> > + rdev->pdev->no_64bit_msi = true;
> >> > + }
> >> > +
> >> > /* force MSI on */
> >> > if (radeon_msi == 1)
> >> > return true;
> >> >
> >> >
> >> >
> >
> >
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] powerpc: Fix warning reported by verify_cpu_node_mapping()
From: Benjamin Herrenschmidt @ 2014-10-02 22:23 UTC (permalink / raw)
To: Nishanth Aravamudan; +Cc: paulus, linuxppc-dev, Nathan Fontenot, Li Zhong
In-Reply-To: <20141002215305.GA20262@linux.vnet.ibm.com>
On Thu, 2014-10-02 at 14:53 -0700, Nishanth Aravamudan wrote:
> On 03.10.2014 [07:28:19 +1000], Benjamin Herrenschmidt wrote:
> > On Thu, 2014-10-02 at 14:13 -0700, Nishanth Aravamudan wrote:
> > > Ben & Michael,
> > >
> > > What's the status of these patches?
> >
> > Waiting for somebody to review them ? :-)
>
> Heh, I acked them about a month ago, I can give a stronger Reviewed-by,
> if you prefer?
Nah, nothing other than having me or mpe pick them up. I've been letting
Michael handle this merge window, so maybe he should take it :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH V7 00/17] Enable SRIOV on POWER8
From: Gavin Shan @ 2014-10-02 23:38 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Wei Yang, Benjamin Herrenschmidt, linux-pci@vger.kernel.org,
Gavin Shan, Guo Chao, Mike Qiu, linuxppc-dev
In-Reply-To: <20141002155943.GA19392@google.com>
On Thu, Oct 02, 2014 at 09:59:43AM -0600, Bjorn Helgaas wrote:
>On Wed, Aug 20, 2014 at 11:35:46AM +0800, Wei Yang wrote:
>> On Tue, Aug 19, 2014 at 10:12:27PM -0500, Bjorn Helgaas wrote:
>> >On Tue, Aug 19, 2014 at 9:34 PM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>> >> On Tue, Aug 19, 2014 at 03:19:42PM -0600, Bjorn Helgaas wrote:
>> >>>On Thu, Jul 24, 2014 at 02:22:10PM +0800, Wei Yang wrote:
>> >>>> This patch set enables the SRIOV on POWER8.
>> >>>>
>> >>>> The gerneral idea is put each VF into one individual PE and allocate required
>> >>>> resources like DMA/MSI.
>> >>>>
>> >>>> One thing special for VF PE is we use M64BT to cover the IOV BAR. M64BT is one
>> >>>> hardware on POWER platform to map MMIO address to PE. By using M64BT, we could
>> >>>> map one individual VF to a VF PE, which introduce more flexiblity to users.
>> >>>>
>> >>>> To achieve this effect, we need to do some hack on pci devices's resources.
>> >>>> 1. Expand the IOV BAR properly.
>> >>>> Done by pnv_pci_ioda_fixup_iov_resources().
>> >>>> 2. Shift the IOV BAR properly.
>> >>>> Done by pnv_pci_vf_resource_shift().
>> >>>> 3. IOV BAR alignment is the total size instead of an individual size on
>> >>>> powernv platform.
>> >>>> Done by pnv_pcibios_sriov_resource_alignment().
>> >>>> 4. Take the IOV BAR alignment into consideration in the sizing and assigning.
>> >>>> This is achieved by commit: "PCI: Take additional IOV BAR alignment in
>> >>>> sizing and assigning"
>> >>>>
>> >>>> Test Environment:
>> >>>> The SRIOV device tested is Emulex Lancer and Mellanox ConnectX-3 on
>> >>>> POWER8.
>> >>>>
>> >>>> Examples on pass through a VF to guest through vfio:
>> >>>> 1. install necessary modules
>> >>>> modprobe vfio
>> >>>> modprobe vfio-pci
>> >>>> 2. retrieve the iommu_group the device belongs to
>> >>>> readlink /sys/bus/pci/devices/0000:06:0d.0/iommu_group
>> >>>> ../../../../kernel/iommu_groups/26
>> >>>> This means it belongs to group 26
>> >>>> 3. see how many devices under this iommu_group
>> >>>> ls /sys/kernel/iommu_groups/26/devices/
>> >>>> 4. unbind the original driver and bind to vfio-pci driver
>> >>>> echo 0000:06:0d.0 > /sys/bus/pci/devices/0000:06:0d.0/driver/unbind
>> >>>> echo 1102 0002 > /sys/bus/pci/drivers/vfio-pci/new_id
>> >>>> Note: this should be done for each device in the same iommu_group
>> >>>> 5. Start qemu and pass device through vfio
>> >>>> /home/ywywyang/git/qemu-impreza/ppc64-softmmu/qemu-system-ppc64 \
>> >>>> -M pseries -m 2048 -enable-kvm -nographic \
>> >>>> -drive file=/home/ywywyang/kvm/fc19.img \
>> >>>> -monitor telnet:localhost:5435,server,nowait -boot cd \
>> >>>> -device "spapr-pci-vfio-host-bridge,id=CXGB3,iommu=26,index=6"
>> >>>>
>> >>>> Verify this is the exact VF response:
>> >>>> 1. ping from a machine in the same subnet(the broadcast domain)
>> >>>> 2. run arp -n on this machine
>> >>>> 9.115.251.20 ether 00:00:c9:df:ed:bf C eth0
>> >>>> 3. ifconfig in the guest
>> >>>> # ifconfig eth1
>> >>>> eth1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
>> >>>> inet 9.115.251.20 netmask 255.255.255.0 broadcast 9.115.251.255
>> >>>> inet6 fe80::200:c9ff:fedf:edbf prefixlen 64 scopeid 0x20<link>
>> >>>> ether 00:00:c9:df:ed:bf txqueuelen 1000 (Ethernet)
>> >>>> RX packets 175 bytes 13278 (12.9 KiB)
>> >>>> RX errors 0 dropped 0 overruns 0 frame 0
>> >>>> TX packets 58 bytes 9276 (9.0 KiB)
>> >>>> TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
>> >>>> 4. They have the same MAC address
>> >>>>
>> >>>> Note: make sure you shutdown other network interfaces in guest.
>> >>>>
>> >>>> ---
>> >>>> v6 -> v7:
>> >>>> 1. add IORESOURCE_ARCH flag for IOV BAR on powernv platform.
>> >>>> 2. when IOV BAR has IORESOURCE_ARCH flag, the size is retrieved from
>> >>>> hardware directly. If not, calculate as usual.
>> >>>> 3. reorder the patch set, group them by subsystem:
>> >>>> PCI, powerpc, powernv
>> >>>> 4. rebase it on 3.16-rc6
>> >>>
>> >>>This doesn't apply for me on v3.16-rc6:
>> >>>
>> >>> 02:48:57 ~/linux$ stg rebase v3.16-rc6
>> >>> Checking for changes in the working directory ... done
>> >>> Rebasing to "v3.16-rc6" ... done
>> >>> No patches applied
>> >>> 02:49:14 ~/linux$ stg import -M --sign m/wy
>> >>> Checking for changes in the working directory ... done
>> >>> Importing patch "pci-iov-export-interface-for" ... done
>> >>> Importing patch "pci-iov-get-vf-bar-size-from" ... done
>> >>> Importing patch "pci-add-weak" ... done
>> >>> Importing patch "pci-take-additional-iov-bar" ... done
>> >>> Importing patch "powerpc-pci-don-t-unset-pci" ... done
>> >>> Importing patch "powerpc-pci-define" ... done
>> >>> Importing patch "powrepc-pci-refactor-pci_dn" ... done
>> >>> Importing patch "powerpc-powernv-use-pci_dn-in" ... error: patch failed:
>> >>> arch/powerpc/platforms/powernv/pci.c:376
>> >>> error: arch/powerpc/platforms/powernv/pci.c: patch does not apply
>> >>> stg import: Diff does not apply cleanly
>> >>>
>> >>>What am I missing?
>> >>>
>> >>>I assume you intend these all to go through my tree just to keep them all
>> >>>together. The ideal rebase target for me would be v3.17-rc1.
>> >>
>> >> Ok, I will rebase it on v3.17-rc1 upstream. While I guess the conflict is due
>> >> to some patches from Gavin, which is not merged at that moment. I will make
>> >> sure it applies to v3.17-rc1.
>> >
>> >I tried applying them on v3.16-rc6 as well as on every change to
>> >arch/powerpc/platforms/powernv/pci.c between v3.16-rc6 and v3.17-rc1,
>> >and none applied cleanly. Patches you post should be based on some
>> >upstream tag, not on something that includes unmerged patches.
>>
>> Sorry about this, I will pay attention to this next time.
>
>I haven't seen any more on this series, and I'm assuming you'll post a
>rebased series (maybe you're waiting for v3.18-rc1?). I'm just checking to
>make sure you're not waiting for something from me...
>
Wei Yang is on vacation and he might not see your reply and response in time.
As discussed with Wei Yang offline, he was waiting for 3.18.rc1 to rebase and
send a new version out for comments.
Thanks,
Gavin
>Bjorn
>
^ permalink raw reply
* Re: [PATCH] [RFC] powerpc/vphn: fix endian issue in NUMA device node code
From: Nishanth Aravamudan @ 2014-10-02 23:43 UTC (permalink / raw)
To: Greg Kurz; +Cc: linuxppc-dev
In-Reply-To: <20141002210621.5615.97402.stgit@bahia.local>
On 02.10.2014 [23:59:15 +0200], Greg Kurz wrote:
> The associativity domain numbers are obtained from the hypervisor through
> registers and written into memory by the guest: the packed array passed to
> vphn_unpack_associativity() is then native-endian, unlike what was assumed
> in the following commit:
>
> commit b08a2a12e44eaec5024b2b969f4fcb98169d1ca3
> Author: Alistair Popple <alistair@popple.id.au>
> Date: Wed Aug 7 02:01:44 2013 +1000
>
> powerpc: Make NUMA device node code endian safe
>
> If a CPU home node changes, the topology gets filled with
> bogus values. This leads to severe performance breakdowns.
>
> This patch does two things:
> - extract values from the packed array with shifts, in order to be endian
> neutral
> - convert the resulting values to be32 as expected
>
> Suggested-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>
> I could test this code in a userland program and obtain the same
> result in little and big endian. If the 64-bit packed values
> are:
>
> 0x8001ffffffff0000
> 0x112200003344ffff
> 0xffff000055660000
> 0x7788000099aaffff
> 0xffff8002ffffffff
> 0xffffffffffffffff
>
> then the unpacked array is:
>
> 0x00000007
> 0x00000001
> 0xffffffff
> 0x00001122
> 0x00003344
> 0xffffffff
> 0x00005566
> 0x00007788
> 0x000099aa
> 0xffffffff
> 0x00000002
> 0xffffffff
> 0xffffffff
>
> I could not test in a PowerVM guest though, hence the RFC.
>
> --
> Greg
>
> arch/powerpc/mm/numa.c | 50 ++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index b835bf0..06af179 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1369,38 +1369,68 @@ static int update_cpu_associativity_changes_mask(void)
> #define VPHN_ASSOC_BUFSIZE (6*sizeof(u64)/sizeof(u32) + 1)
>
> /*
> + * The associativity values are either 16-bit (VPHN_FIELD_MSB) or 32-bit (data
> + * or VPHN_FIELD_UNUSED). We hence need to parse the packed array into 16-bit
> + * chunks. Let's do that with bit shifts to be endian neutral.
> + *
> + * --- 16-bit chunks -->
> + * _________________________
> + * | 0 | 1 | 2 | 3 | packed[0]
> + * -------------------------
> + * _________________________
> + * | 4 | 5 | 6 | 7 | packed[1]
> + * -------------------------
> + * ...
> + * _________________________
> + * | 20 | 21 | 22 | 23 | packed[5]
> + * -------------------------
> + *
> + * >>48 >>32 >>16 >>0 <-- needed bit shift
> + *
> + * We only care for the 2 lower bits of the chunk index to compute the shift.
> + */
> +static inline u16 read_vphn_chunk(const long *packed, unsigned int i)
> +{
> + return packed[i >> 2] >> ((~i & 3) << 4);
This is some excellent magic and the comment *should* be sufficient, but
maybe an example would be good? i is the index we want to read from in
the packed array?
> +}
> +
> +/*
> * Convert the associativity domain numbers returned from the hypervisor
> * to the sequence they would appear in the ibm,associativity property.
> */
> static int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
> {
> - int i, nr_assoc_doms = 0;
> + unsigned int i, j, nr_assoc_doms = 0;
> const __be16 *field = (const __be16 *) packed;
>
> #define VPHN_FIELD_UNUSED (0xffff)
> #define VPHN_FIELD_MSB (0x8000)
> #define VPHN_FIELD_MASK (~VPHN_FIELD_MSB)
>
> - for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
> - if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
> + for (i = 1, j = 0; i < VPHN_ASSOC_BUFSIZE; i++) {
> + u16 field = read_vphn_chunk(packed, j);
Maybe I'm super dense here, but isn't there are already a variable named
field in this context? With a different annotation?
> +
> + if (field == VPHN_FIELD_UNUSED) {
> /* All significant fields processed, and remaining
> * fields contain the reserved value of all 1's.
> * Just store them.
> */
> - unpacked[i] = *((__be32 *)field);
> - field += 2;
> + unpacked[i] = (VPHN_FIELD_UNUSED << 16 |
> + VPHN_FIELD_UNUSED);
> + j += 2;
> } else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
Why do we need to do be16_to_cpup(field) here if field is now a u16? Or
more explicitly, if we don't performe be16_to_cpup(field) anywhere else
in this function now, why do we need to here?
> /* Data is in the lower 15 bits of this field */
> - unpacked[i] = cpu_to_be32(
> - be16_to_cpup(field) & VPHN_FIELD_MASK);
> - field++;
> + unpacked[i] = cpu_to_be32(field & VPHN_FIELD_MASK);
> + j++;
> nr_assoc_doms++;
> } else {
> /* Data is in the lower 15 bits of this field
> * concatenated with the next 16 bit field
> */
> - unpacked[i] = *((__be32 *)field);
> - field += 2;
> + unpacked[i] =
> + cpu_to_be32((u32) field << 16 |
> + read_vphn_chunk(packed, j + 1));
> + j += 2;
> nr_assoc_doms++;
> }
> }
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] powerpc: Fix warning reported by verify_cpu_node_mapping()
From: Michael Ellerman @ 2014-10-03 0:50 UTC (permalink / raw)
To: Nishanth Aravamudan; +Cc: paulus, linuxppc-dev, Li Zhong, Nathan Fontenot
In-Reply-To: <20141002211305.GB12862@linux.vnet.ibm.com>
On Thu, 2014-10-02 at 14:13 -0700, Nishanth Aravamudan wrote:
> Ben & Michael,
>
> What's the status of these patches?
Been in my next for a week :)
https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/log/?h=next
cheers
^ permalink raw reply
* [PATCH 1/3] tools/perf: Fix error message
From: Sukadev Bhattiprolu @ 2014-10-03 0:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, ak, Jiri Olsa, peterz
Cc: Anton Blanchard, linuxppc-dev, linux-kernel
Sometimes, eg: with a stripped vmlinux, we can open the file but
cannot load any symbols from it. Update error message accordingly
so the users can better understand the error.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
tools/perf/util/map.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index b709059..c478092 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -267,7 +267,7 @@ int map__load(struct map *map, symbol_filter_t filter)
pr_warning("%s with build id %s not found",
name, sbuild_id);
} else
- pr_warning("Failed to open %s", name);
+ pr_warning("Failed to load symbols for DSO %s", name);
pr_warning(", continuing without symbols\n");
return -1;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3] tools/perf: Rename variables for clarity
From: Sukadev Bhattiprolu @ 2014-10-03 0:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, ak, Jiri Olsa, peterz
Cc: Anton Blanchard, linuxppc-dev, linux-kernel
In-Reply-To: <1412297616-14179-1-git-send-email-sukadev@linux.vnet.ibm.com>
The dso__load* functions return the number symbols they were able
to load or -1 in case of error.
But it is a bit confusing to determine 'if (err > 0)' indicates success
or failure and we have to step several functions deep to find that out.
Rename the variable 'err' so it is hopefully easier to understand.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
tools/perf/util/symbol.c | 50 ++++++++++++++++++++++++------------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index be84f7a..9b66e27 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1467,7 +1467,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
const char *vmlinux, bool vmlinux_allocated,
symbol_filter_t filter)
{
- int err = -1;
+ int nsyms = -1;
struct symsrc ss;
char symfs_vmlinux[PATH_MAX];
enum dso_binary_type symtab_type;
@@ -1485,10 +1485,10 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
return -1;
- err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
+ nsyms = dso__load_sym(dso, map, &ss, &ss, filter, 0);
symsrc__destroy(&ss);
- if (err > 0) {
+ if (nsyms > 0) {
if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
dso->binary_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
else
@@ -1498,13 +1498,13 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
pr_debug("Using %s for symbols\n", symfs_vmlinux);
}
- return err;
+ return nsyms;
}
int dso__load_vmlinux_path(struct dso *dso, struct map *map,
symbol_filter_t filter)
{
- int i, err = 0;
+ int i, nsyms = 0;
char *filename;
pr_debug("Looking at the vmlinux_path (%d entries long)\n",
@@ -1512,19 +1512,19 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map,
filename = dso__build_id_filename(dso, NULL, 0);
if (filename != NULL) {
- err = dso__load_vmlinux(dso, map, filename, true, filter);
- if (err > 0)
+ nsyms = dso__load_vmlinux(dso, map, filename, true, filter);
+ if (nsyms > 0)
goto out;
free(filename);
}
for (i = 0; i < vmlinux_path__nr_entries; ++i) {
- err = dso__load_vmlinux(dso, map, vmlinux_path[i], false, filter);
- if (err > 0)
+ nsyms = dso__load_vmlinux(dso, map, vmlinux_path[i], false, filter);
+ if (nsyms > 0)
break;
}
out:
- return err;
+ return nsyms;
}
static int find_matching_kcore(struct map *map, char *dir, size_t dir_sz)
@@ -1634,7 +1634,7 @@ proc_kallsyms:
static int dso__load_kernel_sym(struct dso *dso, struct map *map,
symbol_filter_t filter)
{
- int err;
+ int nsyms;
const char *kallsyms_filename = NULL;
char *kallsyms_allocated_filename = NULL;
/*
@@ -1663,9 +1663,9 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map,
}
if (!symbol_conf.ignore_vmlinux && vmlinux_path != NULL) {
- err = dso__load_vmlinux_path(dso, map, filter);
- if (err > 0)
- return err;
+ nsyms = dso__load_vmlinux_path(dso, map, filter);
+ if (nsyms > 0)
+ return nsyms;
}
/* do not try local files if a symfs was given */
@@ -1679,25 +1679,25 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map,
kallsyms_filename = kallsyms_allocated_filename;
do_kallsyms:
- err = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
- if (err > 0)
+ nsyms = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
+ if (nsyms > 0)
pr_debug("Using %s for symbols\n", kallsyms_filename);
free(kallsyms_allocated_filename);
- if (err > 0 && !dso__is_kcore(dso)) {
+ if (nsyms > 0 && !dso__is_kcore(dso)) {
dso->binary_type = DSO_BINARY_TYPE__KALLSYMS;
dso__set_long_name(dso, "[kernel.kallsyms]", false);
map__fixup_start(map);
map__fixup_end(map);
}
- return err;
+ return nsyms;
}
static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
symbol_filter_t filter)
{
- int err;
+ int nsyms;
const char *kallsyms_filename = NULL;
struct machine *machine;
char path[PATH_MAX];
@@ -1715,10 +1715,10 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
* Or use file guest_kallsyms inputted by user on commandline
*/
if (symbol_conf.default_guest_vmlinux_name != NULL) {
- err = dso__load_vmlinux(dso, map,
+ nsyms = dso__load_vmlinux(dso, map,
symbol_conf.default_guest_vmlinux_name,
false, filter);
- return err;
+ return nsyms;
}
kallsyms_filename = symbol_conf.default_guest_kallsyms;
@@ -1729,10 +1729,10 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
kallsyms_filename = path;
}
- err = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
- if (err > 0)
+ nsyms = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
+ if (nsyms > 0)
pr_debug("Using %s for symbols\n", kallsyms_filename);
- if (err > 0 && !dso__is_kcore(dso)) {
+ if (nsyms > 0 && !dso__is_kcore(dso)) {
dso->binary_type = DSO_BINARY_TYPE__GUEST_KALLSYMS;
machine__mmap_name(machine, path, sizeof(path));
dso__set_long_name(dso, strdup(path), true);
@@ -1740,7 +1740,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map,
map__fixup_end(map);
}
- return err;
+ return nsyms;
}
static void vmlinux_path__exit(void)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3] tools/perf: Fix error reporting
From: Sukadev Bhattiprolu @ 2014-10-03 0:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, ak, Jiri Olsa, peterz
Cc: Anton Blanchard, linuxppc-dev, linux-kernel
In-Reply-To: <1412297616-14179-1-git-send-email-sukadev@linux.vnet.ibm.com>
If user explicitly specifies a vmlinux or kallsyms file on the command
line and the specified file doesn't yield any symbols, print a warning
message.
$ perf report -kallsyms
No kernel symbols in the vmlinux 'allsyms'?
Failed to load symbols for DSO [kernel.kallsyms], continuing
without symbols
This could help user better recognize the typo -kallsyms v. --kallsyms.
It would also help if the user points to a stripped/invalid vmlinux or
an invalid kallsyms.
With a stripped vmlinux:
$ perf report -k /tmp/vmlinux
No kernel symbols in the vmlinux '/tmp/vmlinux'?
Failed to load symbols for DSO [kernel.kallsyms], continuing
without symbols
and with perf top
$ perf top -k /tmp/vmlinux
No kernel symbols in the vmlinux '/tmp/vmlinux'?
/tmp/vmlinux with build id f43f4e78d3afac6492dcae52cd756394247997d6
not found, continuing without symbols
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
tools/perf/util/symbol.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9b66e27..ad5baa4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1658,8 +1658,13 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map,
}
if (!symbol_conf.ignore_vmlinux && symbol_conf.vmlinux_name != NULL) {
- return dso__load_vmlinux(dso, map, symbol_conf.vmlinux_name,
+ nsyms = dso__load_vmlinux(dso, map, symbol_conf.vmlinux_name,
false, filter);
+ if (nsyms <= 0) {
+ pr_warning("No kernel symbols in the vmlinux '%s'?\n",
+ symbol_conf.vmlinux_name);
+ }
+ return nsyms;
}
if (!symbol_conf.ignore_vmlinux && vmlinux_path != NULL) {
@@ -1682,6 +1687,10 @@ do_kallsyms:
nsyms = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
if (nsyms > 0)
pr_debug("Using %s for symbols\n", kallsyms_filename);
+ else if (symbol_conf.kallsyms_name) {
+ pr_warning("No kernel symbols in the kallsyms '%s'?\n",
+ kallsyms_filename);
+ }
free(kallsyms_allocated_filename);
if (nsyms > 0 && !dso__is_kcore(dso)) {
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v3] powerpc/iommu/ddw: Fix endianness
From: Anton Blanchard @ 2014-10-03 4:21 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: devicetree, linuxppc-dev, linux-kernel
In-Reply-To: <1411627167-22927-1-git-send-email-aik@ozlabs.ru>
Hi Alexey,
> rtas_call() accepts and returns values in CPU endianness.
> The ddw_query_response and ddw_create_response structs members are
> defined and treated as BE but as they are passed to rtas_call() as
> (u32 *) and they get byteswapped automatically, the data is
> CPU-endian. This fixes ddw_query_response and ddw_create_response
> definitions and use.
>
> of_read_number() is designed to work with device tree cells - it
> assumes the input is big-endian and returns data in CPU-endian.
> However due to the ddw_create_response struct fix, create.addr_hi/lo
> are already CPU-endian so do not byteswap them.
>
> ddw_avail is a pointer to the "ibm,ddw-applicable" property which
> contains 3 cells which are big-endian as it is a device tree.
> rtas_call() accepts a RTAS token in CPU-endian. This makes use of
> of_property_read_u32_array to byte swap and avoid the need for a
> number of be32_to_cpu calls.
>
> Cc: stable@vger.kernel.org # v3.13
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Anton Blanchard <anton@samba.org>
> [aik: folded Anton's patch with of_property_read_u32_array]
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Thanks for updating, looks good. Could we make it clear the bug is
present in 3.13-3.17 with:
Cc: stable@vger.kernel.org # v3.13+
Acked-by: Anton Blanchard <anton@samba.org>
Anton
^ 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