* Re: [PATCH 4/4] powerpc: Move htab_remove_mapping function prototype into header file
From: Stephen Rothwell @ 2014-08-22 15:29 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev
In-Reply-To: <1408488921-19187-4-git-send-email-anton@samba.org>
[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]
Hi Anton,
On Wed, 20 Aug 2014 08:55:21 +1000 Anton Blanchard <anton@samba.org> wrote:
>
> A recent patch added a function prototype for htab_remove_mapping in
> c code. Fix it.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> arch/powerpc/include/asm/mmu-hash64.h | 2 ++
> arch/powerpc/mm/init_64.c | 3 ---
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
> index d765144..92bc3a6 100644
> --- a/arch/powerpc/include/asm/mmu-hash64.h
> +++ b/arch/powerpc/include/asm/mmu-hash64.h
> @@ -342,6 +342,8 @@ extern void hash_failure_debug(unsigned long ea, unsigned long access,
> extern int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
> unsigned long pstart, unsigned long prot,
> int psize, int ssize);
> +int htab_remove_mapping(unsigned long vstart, unsigned long vend,
> + int psize, int ssize);
Please be consistent about "extern" use (unless this file is already
inconsistent, I guess). (I know that the current trend is to remove
"extern" in header files - I just happen to disagree with that
trend. :-))
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects
From: Scott Wood @ 2014-08-22 17:51 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: Greg Kroah-Hartman, Aaron Sierra, linuxppc-dev, Arnd Bergmann
In-Reply-To: <53F755A0.8020504@freescale.com>
On Fri, 2014-08-22 at 20:07 +0530, Prabhakar Kushwaha wrote:
> Sorry Scott for late reply,
>
> Please find my reply in-lined
>
>
> On 8/21/2014 4:51 AM, Scott Wood wrote:
> > On Wed, 2014-08-20 at 09:05 +0530, Prabhakar Kushwaha wrote:
> >> On 8/20/2014 5:38 AM, Scott Wood wrote:
> >>> On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
> >>>> Freescale's QorIQ T Series processors support 8 IFC chip selects
> >>>> within a memory map backward compatible with previous P Series
> >>>> processors which supported only 4 chip selects.
> >>>>
> >>>> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> >>>> ---
> >>>> include/linux/fsl_ifc.h | 10 +++++-----
> >>>> 1 file changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> >>>> index 84d60cb..62762ff 100644
> >>>> --- a/include/linux/fsl_ifc.h
> >>>> +++ b/include/linux/fsl_ifc.h
> >>>> @@ -29,7 +29,7 @@
> >>>> #include <linux/of_platform.h>
> >>>> #include <linux/interrupt.h>
> >>>>
> >>>> -#define FSL_IFC_BANK_COUNT 4
> >>>> +#define FSL_IFC_BANK_COUNT 8
> >>> First please modify fsl_ifc_nand.c to limit itself to the number of
> >>> banks it dynamically determines are present based on the IFC version.
> >>>
> >>>
> >> Number of available bank/chip select are defined by SoC and it is
> >> independent of SoC.
> > Do you mean defined by the SoC and independent of the IFC version?
>
> IFC v 1.0.0 supports 4 Chip Select.
> IFC v 1.1.0 onwards, IFC supports 8 chip select.
>
> But SoC finally defines number of chip select coming out of SoC. Like
> LS1021A with IFC ver 1.4.0 have only 7 Chip Select.
What matters here is whether the registers are implemented, not whether
a chip select is pinned out -- so use the IFC version.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
From: Aaron Sierra @ 2014-08-22 17:54 UTC (permalink / raw)
To: Scott Wood; +Cc: Minghuan Lian, linuxppc-dev
In-Reply-To: <1408658506.2395.11.camel@snotra.buserror.net>
----- Original Message -----
> From: "Scott Wood" <scottwood@freescale.com>
> Sent: Thursday, August 21, 2014 5:01:46 PM
>
> On Thu, 2014-08-21 at 16:54 -0500, Aaron Sierra wrote:
> > ----- Original Message -----
> > > From: "Scott Wood" <scottwood@freescale.com>
> > > Sent: Thursday, August 21, 2014 4:19:56 PM
> > >
> > > On Wed, 2014-08-20 at 18:51 -0500, Aaron Sierra wrote:
> > > > @@ -520,9 +520,22 @@ int fsl_add_bridge(struct platform_device *pdev,
> > > > int
> > > > is_primary)
> > > > goto no_bridge;
> > > >
> > > > } else {
> > > > - /* For PCI read PROG to identify controller mode */
> > > > - early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
> > > > - if ((progif & 1) == 1)
> > > > + u16 master;
> > > > +
> > > > + /*
> > > > + * If the controller is PCI-X, then Host mode refers to a
> > > > + * bridge that drives the PCI-X initialization pattern to
> > > > + * indicate bus operating mode/frequency to devices on the bus.
> > > > + * Some hardware (specifically PrPMC modules) are Agents, since
> > > > + * the mezzanine carrier is responsible for driving the
> > > > + * pattern, but they still may perform bus enumeration.
> > > > + *
> > > > + * Allow the bridge to be used for enumeration, if hardware
> > > > + * strapping (Host mode) or firmware (Agent mode) has enabled
> > > > + * bus mastering.
> > > > + */
> > > > + early_read_config_word(hose, 0, 0, PCI_COMMAND, &master);
> > > > + if (!(master & PCI_COMMAND_MASTER))
> > > > goto no_bridge;
> > > > }
> > >
> > > Why wouldn't a normal PCI agent be able to bus master?
> > >
> > > -Scott
> > >
> >
> > Short answer:
> >
> > Simply because the hardware strapping for Host/Agent determines the
> > default state of the Bus Master bit in the Command register. Without
> > that bit being set, an Agent won't be able to send the PCI cycles
> > necessary to enumerate the bus.
>
> But what if the host has already set that bit before Linux boots?
That's a very good point. I think that concern can be addressed by looking
for another telltale sign of enumeration, whether an address has been
assigned to the bridge's BAR 0 (PCSRBAR).
> > Long answer:
> >
> > I think there was an assumption in the patch that introduced the
> > regression that Host and Agent in conventional PCI and PCI-X are more
> > equivalent to PCIe Root Complex and Endpoint than they really are.
> >
> > I think the purpose of Minghuan's patch was to not attempt to enumerate
> > the bus with a bridge that simply cannot do it. A PCIe Endpoint cannot
> > enumerate the bus and a PCI/PCI-X device that cannot master cycles on the
> > bus will not be able to enumerate the bus either.
>
> No, the point is also to not enumerate the bus in cases where we
> shouldn't. We don't want to step on the host's toes.
Agreed.
> > The hardware strapping for Host/Agent mode determines the default state
> > of the Bus Master bit in the Command register.
> >
> > In our more specialized, but still industry standardized, environment our
> > firmware must detect whether our board should be the bus's sole enumerator
> > and set the Bus Master bit accordingly.
> >
> > My comment in the code still mentions Host and Agent mode, simply because
> > the code it is replacing based its decision on the mode.
>
> I understand why you need to do this -- I just don't think this is a
> reliable way of detecting that you're in that situation. How about a
> kernel command line setting?
I'd like to avoid requiring a kernel command-line option for this.
Instead, I propose checking for Agent mode like Minghuan's patch, but
prohibiting enumeration only if Bus Master is not enabled OR the address
portion of PCSRBAR is non-zero (indicating another device has enumerated
the bus).
Pseudo code:
if (agent && (!master || enumerated))
goto no_bridge;
-Aaron
^ permalink raw reply
* Re: [PATCH] ibmveth: Fix endian issues with rx_no_buffer statistic
From: David Miller @ 2014-08-22 18:28 UTC (permalink / raw)
To: anton; +Cc: paulus, santil, linuxppc-dev, netdev
In-Reply-To: <20140822113652.7e4bdc78@kryten>
From: Anton Blanchard <anton@samba.org>
Date: Fri, 22 Aug 2014 11:36:52 +1000
> Hidden away in the last 8 bytes of the buffer_list page is a solitary
> statistic. It needs to be byte swapped or else ethtool -S will
> produce numbers that terrify the user.
>
> Since we do this in multiple places, create a helper function with a
> comment explaining what is going on.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
Applied, thanks Anton.
^ permalink raw reply
* Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
From: Scott Wood @ 2014-08-22 18:36 UTC (permalink / raw)
To: Aaron Sierra; +Cc: Minghuan Lian, linuxppc-dev
In-Reply-To: <964109192.130707.1408730093575.JavaMail.zimbra@xes-inc.com>
On Fri, 2014-08-22 at 12:54 -0500, Aaron Sierra wrote:
> ----- Original Message -----
> > From: "Scott Wood" <scottwood@freescale.com>
> > Sent: Thursday, August 21, 2014 5:01:46 PM
> >
> > On Thu, 2014-08-21 at 16:54 -0500, Aaron Sierra wrote:
> > > ----- Original Message -----
> > > > From: "Scott Wood" <scottwood@freescale.com>
> > > > Sent: Thursday, August 21, 2014 4:19:56 PM
> > > >
> > > > Why wouldn't a normal PCI agent be able to bus master?
> > > >
> > > > -Scott
> > > >
> > >
> > > Short answer:
> > >
> > > Simply because the hardware strapping for Host/Agent determines the
> > > default state of the Bus Master bit in the Command register. Without
> > > that bit being set, an Agent won't be able to send the PCI cycles
> > > necessary to enumerate the bus.
> >
> > But what if the host has already set that bit before Linux boots?
>
> That's a very good point. I think that concern can be addressed by looking
> for another telltale sign of enumeration, whether an address has been
> assigned to the bridge's BAR 0 (PCSRBAR).
I don't see how that's any different. The host may or may not have
assigned an address.
> > I understand why you need to do this -- I just don't think this is a
> > reliable way of detecting that you're in that situation. How about a
> > kernel command line setting?
>
> I'd like to avoid requiring a kernel command-line option for this.
It's hardware description, so you could use a device tree property.
-Scott
^ permalink raw reply
* [PATCH 1/5] kexec: Fix make headers_check
From: Geoff Levand @ 2014-08-22 18:39 UTC (permalink / raw)
To: Eric Biederman; +Cc: Andrew Morton, kexec, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1408731991.git.geoff@infradead.org>
Remove the unneded declaration for a kexec_load() routine.
Fixes errors like these when running 'make headers_check':
include/uapi/linux/kexec.h: userspace cannot reference function or variable defined in the kernel
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
include/uapi/linux/kexec.h | 6 ------
1 file changed, 6 deletions(-)
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 6925f5b..99048e5 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -55,12 +55,6 @@ struct kexec_segment {
size_t memsz;
};
-/* Load a new kernel image as described by the kexec_segment array
- * consisting of passed number of segments at the entry-point address.
- * The flags allow different useage types.
- */
-extern int kexec_load(void *, size_t, struct kexec_segment *,
- unsigned long int);
#endif /* __KERNEL__ */
#endif /* _UAPILINUX_KEXEC_H */
--
1.9.1
^ permalink raw reply related
* [PATCH 0/5] kexec: minor fixups and enhancements
From: Geoff Levand @ 2014-08-22 18:39 UTC (permalink / raw)
To: Eric Biederman; +Cc: Andrew Morton, kexec, linuxppc-dev, linux-kernel
Hi,
Here are a few minor fixups and enhancements for kexec support.
Patch 3 and 4 that add preprocessor macros for the kimage list flags are
ones that I use in the arm64 kexec support I am working on, so it would
be nice for those to go in.
Please consider.
-Geoff
The following changes since commit 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9:
Linux 3.17-rc1 (2014-08-16 10:40:26 -0600)
are available in the git repository at:
git://git.linaro.org/people/geoff.levand/linux-kexec.git for-kexec
for you to fetch changes up to dc3c63e69815e00dce5454bd50e2991e9ea33f64:
powerpc/kexec: Use global IND_FLAGS macro (2014-08-22 11:15:18 -0700)
----------------------------------------------------------------
Geoff Levand (5):
kexec: Fix make headers_check
kexec: Simplify conditional
kexec: Add bit definitions for kimage entry flags
kexec: Add IND_FLAGS macro
powerpc/kexec: Use global IND_FLAGS macro
arch/powerpc/kernel/machine_kexec_64.c | 2 --
include/linux/kexec.h | 20 ++++++++++++++++----
include/uapi/linux/kexec.h | 6 ------
kernel/kexec.c | 17 ++++++++++-------
4 files changed, 26 insertions(+), 19 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 5/5] powerpc/kexec: Use global IND_FLAGS macro
From: Geoff Levand @ 2014-08-22 18:39 UTC (permalink / raw)
To: Eric Biederman; +Cc: Andrew Morton, kexec, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1408731991.git.geoff@infradead.org>
linux/kexec.h now defines an IND_FLAGS macro. Remove the local powerpc
definition and use the generic one.
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
arch/powerpc/kernel/machine_kexec_64.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 879b3aa..75652a32 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -96,8 +96,6 @@ int default_machine_kexec_prepare(struct kimage *image)
return 0;
}
-#define IND_FLAGS (IND_DESTINATION | IND_INDIRECTION | IND_DONE | IND_SOURCE)
-
static void copy_segments(unsigned long ind)
{
unsigned long entry;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/5] kexec: Simplify conditional
From: Geoff Levand @ 2014-08-22 18:39 UTC (permalink / raw)
To: Eric Biederman; +Cc: Andrew Morton, kexec, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1408731991.git.geoff@infradead.org>
Simplify the code around one of the conditionals in the kexec_load
syscall routine.
The original code was confusing with a redundant check on KEXEC_ON_CRASH
and comments outside of the conditional block. This change switches the
order of the conditional check, and cleans up the comments for the
conditional. There is no functional change to the code.
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
kernel/kexec.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 0b49a0a..d04b56e 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1282,19 +1282,22 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
if (nr_segments > 0) {
unsigned long i;
- /* Loading another kernel to reboot into */
- if ((flags & KEXEC_ON_CRASH) == 0)
- result = kimage_alloc_init(&image, entry, nr_segments,
- segments, flags);
- /* Loading another kernel to switch to if this one crashes */
- else if (flags & KEXEC_ON_CRASH) {
- /* Free any current crash dump kernel before
+ if (flags & KEXEC_ON_CRASH) {
+ /*
+ * Loading another kernel to switch to if this one
+ * crashes. Free any current crash dump kernel before
* we corrupt it.
*/
+
kimage_free(xchg(&kexec_crash_image, NULL));
result = kimage_alloc_init(&image, entry, nr_segments,
segments, flags);
crash_map_reserved_pages();
+ } else {
+ /* Loading another kernel to reboot into. */
+
+ result = kimage_alloc_init(&image, entry, nr_segments,
+ segments, flags);
}
if (result)
goto out;
--
1.9.1
^ permalink raw reply related
* [PATCH 4/5] kexec: Add IND_FLAGS macro
From: Geoff Levand @ 2014-08-22 18:39 UTC (permalink / raw)
To: Eric Biederman; +Cc: Andrew Morton, kexec, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1408731991.git.geoff@infradead.org>
Add a new kexec preprocessor macro IND_FLAGS, which is the bitwise OR of
all the possible kexec IND_ kimage_entry indirection flags.
Having this macro allows for simplified code in the prosessing of the
kexec kimage_entry items.
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
include/linux/kexec.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8c628ca..a4758f9 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -10,6 +10,7 @@
#define IND_INDIRECTION (1 << IND_INDIRECTION_BIT)
#define IND_DONE (1 << IND_DONE_BIT)
#define IND_SOURCE (1 << IND_SOURCE_BIT)
+#define IND_FLAGS (IND_DESTINATION | IND_INDIRECTION | IND_DONE | IND_SOURCE)
#if !defined(__ASSEMBLY__)
--
1.9.1
^ permalink raw reply related
* [PATCH 3/5] kexec: Add bit definitions for kimage entry flags
From: Geoff Levand @ 2014-08-22 18:39 UTC (permalink / raw)
To: Eric Biederman; +Cc: Andrew Morton, kexec, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1408731991.git.geoff@infradead.org>
Define new kexec preprocessor macros IND_*_BIT that define the bit position of
the kimage entry flags. Change the existing IND_* flag macros to be defined as
bit shifts of the corresponding IND_*_BIT macros. Also wrap all C language code
in kexec.h with #if !defined(__ASSEMBLY__) so assembly files can include kexec.h
to get the IND_* and IND_*_BIT macros.
Some CPU instruction sets have tests for bit position which are convenient in
implementing routines that operate on the kimage entry list. The addition of
these bit position macros in a common location will avoid duplicate definitions
and the chance that changes to the IND_* flags will not be propagated to
assembly files.
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
include/linux/kexec.h | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 4b2a0e1..8c628ca 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -1,6 +1,18 @@
#ifndef LINUX_KEXEC_H
#define LINUX_KEXEC_H
+#define IND_DESTINATION_BIT 0
+#define IND_INDIRECTION_BIT 1
+#define IND_DONE_BIT 2
+#define IND_SOURCE_BIT 3
+
+#define IND_DESTINATION (1 << IND_DESTINATION_BIT)
+#define IND_INDIRECTION (1 << IND_INDIRECTION_BIT)
+#define IND_DONE (1 << IND_DONE_BIT)
+#define IND_SOURCE (1 << IND_SOURCE_BIT)
+
+#if !defined(__ASSEMBLY__)
+
#include <uapi/linux/kexec.h>
#ifdef CONFIG_KEXEC
@@ -64,10 +76,6 @@
*/
typedef unsigned long kimage_entry_t;
-#define IND_DESTINATION 0x1
-#define IND_INDIRECTION 0x2
-#define IND_DONE 0x4
-#define IND_SOURCE 0x8
struct kexec_segment {
/*
@@ -312,4 +320,7 @@ struct task_struct;
static inline void crash_kexec(struct pt_regs *regs) { }
static inline int kexec_should_crash(struct task_struct *p) { return 0; }
#endif /* CONFIG_KEXEC */
+
+#endif /* !defined(__ASSEBMLY__) */
+
#endif /* LINUX_KEXEC_H */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
From: Aaron Sierra @ 2014-08-22 19:00 UTC (permalink / raw)
To: Scott Wood; +Cc: Minghuan Lian, linuxppc-dev
In-Reply-To: <1408732591.6510.5.camel@snotra.buserror.net>
----- Original Message -----
> From: "Scott Wood" <scottwood@freescale.com>
> To: "Aaron Sierra" <asierra@xes-inc.com>
> Cc: linuxppc-dev@lists.ozlabs.org, "Minghuan Lian" <Minghuan.Lian@freescale.com>
> Sent: Friday, August 22, 2014 1:36:31 PM
> Subject: Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
>
> On Fri, 2014-08-22 at 12:54 -0500, Aaron Sierra wrote:
> > ----- Original Message -----
> > > From: "Scott Wood" <scottwood@freescale.com>
> > > Sent: Thursday, August 21, 2014 5:01:46 PM
> > >
> > > On Thu, 2014-08-21 at 16:54 -0500, Aaron Sierra wrote:
> > > > ----- Original Message -----
> > > > > From: "Scott Wood" <scottwood@freescale.com>
> > > > > Sent: Thursday, August 21, 2014 4:19:56 PM
> > > > >
> > > > > Why wouldn't a normal PCI agent be able to bus master?
> > > > >
> > > > > -Scott
> > > > >
> > > >
> > > > Short answer:
> > > >
> > > > Simply because the hardware strapping for Host/Agent determines the
> > > > default state of the Bus Master bit in the Command register. Without
> > > > that bit being set, an Agent won't be able to send the PCI cycles
> > > > necessary to enumerate the bus.
> > >
> > > But what if the host has already set that bit before Linux boots?
> >
> > That's a very good point. I think that concern can be addressed by looking
> > for another telltale sign of enumeration, whether an address has been
> > assigned to the bridge's BAR 0 (PCSRBAR).
>
> I don't see how that's any different. The host may or may not have
> assigned an address.
I don't agree with that. If the host has enabled bus mastering for the
device, then it also surely would also have assigned an address to the
always on PCSRBAR during enumeration.
In what sort of environment would a host have enabled bus mastering for
a peripheral device _before_ assigning addresses to its BARs?
> > > I understand why you need to do this -- I just don't think this is a
> > > reliable way of detecting that you're in that situation. How about a
> > > kernel command line setting?
> >
> > I'd like to avoid requiring a kernel command-line option for this.
>
> It's hardware description, so you could use a device tree property.
>
I like this better than a command-line option, but what are you
suggesting? Would you want a device-tree property to gate the logic that
I've proposed?
-Aaron
^ permalink raw reply
* Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
From: Scott Wood @ 2014-08-22 19:53 UTC (permalink / raw)
To: Aaron Sierra; +Cc: Minghuan Lian, linuxppc-dev
In-Reply-To: <1876951015.136131.1408734021445.JavaMail.zimbra@xes-inc.com>
On Fri, 2014-08-22 at 14:00 -0500, Aaron Sierra wrote:
> ----- Original Message -----
> > From: "Scott Wood" <scottwood@freescale.com>
> > To: "Aaron Sierra" <asierra@xes-inc.com>
> > Cc: linuxppc-dev@lists.ozlabs.org, "Minghuan Lian" <Minghuan.Lian@freescale.com>
> > Sent: Friday, August 22, 2014 1:36:31 PM
> > Subject: Re: [PATCH] powerpc: fsl_pci: Fix PCI/PCI-X regression
> >
> > On Fri, 2014-08-22 at 12:54 -0500, Aaron Sierra wrote:
> > > ----- Original Message -----
> > > > From: "Scott Wood" <scottwood@freescale.com>
> > > > Sent: Thursday, August 21, 2014 5:01:46 PM
> > > >
> > > > On Thu, 2014-08-21 at 16:54 -0500, Aaron Sierra wrote:
> > > > > ----- Original Message -----
> > > > > > From: "Scott Wood" <scottwood@freescale.com>
> > > > > > Sent: Thursday, August 21, 2014 4:19:56 PM
> > > > > >
> > > > > > Why wouldn't a normal PCI agent be able to bus master?
> > > > > >
> > > > > > -Scott
> > > > > >
> > > > >
> > > > > Short answer:
> > > > >
> > > > > Simply because the hardware strapping for Host/Agent determines the
> > > > > default state of the Bus Master bit in the Command register. Without
> > > > > that bit being set, an Agent won't be able to send the PCI cycles
> > > > > necessary to enumerate the bus.
> > > >
> > > > But what if the host has already set that bit before Linux boots?
> > >
> > > That's a very good point. I think that concern can be addressed by looking
> > > for another telltale sign of enumeration, whether an address has been
> > > assigned to the bridge's BAR 0 (PCSRBAR).
> >
> > I don't see how that's any different. The host may or may not have
> > assigned an address.
>
> I don't agree with that. If the host has enabled bus mastering for the
> device, then it also surely would also have assigned an address to the
> always on PCSRBAR during enumeration.
>
> In what sort of environment would a host have enabled bus mastering for
> a peripheral device _before_ assigning addresses to its BARs?
It's unusual, but not illegal and doing so shouldn't cause a PCI device
to try to take over the system.
Plus, zero is (arguably) a valid BAR address.
> > > > I understand why you need to do this -- I just don't think this is a
> > > > reliable way of detecting that you're in that situation. How about a
> > > > kernel command line setting?
> > >
> > > I'd like to avoid requiring a kernel command-line option for this.
> >
> > It's hardware description, so you could use a device tree property.
> >
>
> I like this better than a command-line option, but what are you
> suggesting? Would you want a device-tree property to gate the logic that
> I've proposed?
I don't think you need the logic you've proposed if you have a device
tree property -- the property would assert that you are the owner of the
bus despite being an agent.
-Scott
^ permalink raw reply
* Re: [RFC PATCH 0/4] Improve slab consumption with memoryless nodes
From: Andrew Morton @ 2014-08-22 20:32 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
Linux Memory Management List, Paul Mackerras, Tejun Heo,
Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
Anton Blanchard
In-Reply-To: <20140822011011.GF13999@linux.vnet.ibm.com>
On Thu, 21 Aug 2014 18:10:11 -0700 Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
> I know kernel summit is going on, so I'll be patient, but was just
> curious if anyone had any further comments other than Christoph's on the
> naming.
Nope. Please make a decision on the naming, refresh, retest and resend
and I'll get the patches queued up for review and testing.
^ permalink raw reply
* Re: [RFC PATCH v3 1/4] topology: add support for node_to_mem_node() to determine the fallback node
From: Nishanth Aravamudan @ 2014-08-22 21:52 UTC (permalink / raw)
To: Christoph Lameter
Cc: Han Pingtian, Matt Mackall, Pekka Enberg,
Linux Memory Management List, Paul Mackerras, David Rientjes,
Tejun Heo, Andrew Morton, linuxppc-dev, Joonsoo Kim, Wanpeng Li,
Anton Blanchard
In-Reply-To: <20140814200656.GP11121@linux.vnet.ibm.com>
Hi Christoph,
On 14.08.2014 [13:06:56 -0700], Nishanth Aravamudan wrote:
> On 14.08.2014 [09:35:37 -0500], Christoph Lameter wrote:
> > On Wed, 13 Aug 2014, Nishanth Aravamudan wrote:
> >
> > > +++ b/include/linux/topology.h
> > > @@ -119,11 +119,20 @@ static inline int numa_node_id(void)
> > > * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem().
> > > */
> > > DECLARE_PER_CPU(int, _numa_mem_);
> > > +extern int _node_numa_mem_[MAX_NUMNODES];
> >
> > Why are these variables starting with an _ ?
> > Maybe _numa_mem was defined that way because it is typically not defined.
> > We dont do this in other situations.
>
> That's how it was in Joonsoo's patch and I was trying to minimize the
> changes from his version (beyond making it compile). I can of course
> update it to not have a prefixing _ if that's preferred.
Upon reflection, did you mean all of these variables? Would you rather I
submitted a follow-on patch that removed the prefix _? Note that
_node_numa_mem_ is also not defined if !MEMORYLESS_NODES.
-Nish
^ permalink raw reply
* Re: [RFC PATCH powerpc] Fix warning reported by verify_cpu_node_mapping()
From: Nishanth Aravamudan @ 2014-08-22 22:04 UTC (permalink / raw)
To: Li Zhong; +Cc: Michael Ellerman, PowerPC email list, Paul Mackerras
In-Reply-To: <1408673576.2448.6.camel@TP420>
On 22.08.2014 [10:12:56 +0800], Li Zhong wrote:
> On ???, 2014-08-21 at 08:45 -0700, Nishanth Aravamudan wrote:
> > On 21.08.2014 [16:14:02 +0800], Li Zhong wrote:
> > > With commit 2fabf084b, 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 could see something like following reported:
> > > [ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the same
> > > node!
> >
> > You mean you did see this, right? (as opposed to "could" based upon code
> > inspection or something)
>
> Yes, I did see the warnings. Seems I didn't express it precisely in
> English ...
>
> >
> > >
> > > 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()?
> > >
> > > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> >
> > Acked-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
>
> Thank you for the review,
>
> Zhong
>
> >
> > > ---
> > > 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);
> >
> > This is a good change, thanks for catching it. I must have glossed over
> > those messages in my testing, my apologies!
Actually, thinking about this more, do you think it makes more sense to
do:
for_each_present_cpu(cpu) in this loop? That is, at boot, ensure all
present (but possibly offline) CPUs have their NUMA mapping set up. CPUs
that aren't present (but are possible) might trigger other warnings,
right? (e.g., the WARN_ON(1) in numa_setup_cpu)
-Nish
^ permalink raw reply
* Re: [PATCH v2 00/14] Add support for parameterized events from sysfs
From: Sukadev Bhattiprolu @ 2014-08-23 0:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: ak, Michael Ellerman, peterz, linux-kernel, eranian, dev,
Arnaldo Carvalho de Melo, linuxppc-dev, mingo, Anshuman Khandual
In-Reply-To: <20140820141545.GA3242@krava.brq.redhat.com>
Jiri Olsa [jolsa@redhat.com] wrote:
<snip>
| > Description of the sysfs contents when events are parameterized (copied from an
| > included patch):
| >
| > Examples:
| >
| > domain=0x1,offset=0x8,starting_index=phys_cpu
| >
| > In the case of the last example, a value replacing "phys_cpu"
| > would need to be provided by the user selecting the particular
| > event. This is refered to as "event parameterization". All
| > non-numerical values indicate an event parameter.
| >
| > Notes on how perf-list displays parameterized events (and how to use them,
| > again culled from an included patch):
| >
| > PARAMETERIZED EVENTS
| > --------------------
| >
| > Some pmu events listed by 'perf-list' will be displayed with '?' in
| > them. For example:
| >
| > hv_gpci/dtbp_ptitc,phys_processor_idx=?/
| >
| > This means that when provided as an event, a value for
| > phys_processor_idx must also be supplied. For example:
| >
| > perf stat -e 'hv_gpci/dtbp_ptitc,phys_processor_idx=0x2/' ...
|
| hi,
| is the reason for this to document this field for event
| in "events/<event>" file?
We are trying to document that for the parameters that have the ? in
perf list, the parameter must be specified otherwise the event will
not be recognized.
|
| Because once you have the field (phys_processor_idx) defined in
| "formats/phys_processor_idx" you should be able to use it as in
| your example:
|
| perf stat -e 'hv_gpci/dtbp_ptitc,phys_processor_idx=0x2/'
|
| without any changes
For some events 'starting_index' refers to physical processor index
as shown in the sysfs entry:
$ cd /sys/bus/event_source/devices/hv_gpci/events
$ cat dispatch_timebase_by_processor_processor_time_in_timebase_cycles
request=0x10,starting_index=phys_processor_idx,counter_info_version=0x8,length=8,offset=0
and 'perf list' for this entry shows 'starting_index' with a ?
indicating it is a requireed parameter.
IIUC, rather than have the user specify a value for 'phys_processor_idx'
-e hv_gpci/dtbp_ptitc,phys_processor_idx=4/'
we would use following right ?
-e hv_gpci/dtbp_ptitc,starting_index=4/'
If so, I think the interface change makes sense. perf list would also
show 'starting_index=?' for the event.
But in the sysfs entry, rather than show 'starting_index=?', should we
leave it as:
'starting_index=phys_processor_idx'
For some events 'startind_index' refers to a physical processor id
and for others it is virtual processor id. So, showing phys_processor_idx
could serve as a hint.
Michael, Cody, Ingo, Peter, let me know if you agree or have other
comments on the inteface.
Thanks,
Sukadev
|
| thanks,
| jirka
^ permalink raw reply
* Re: [PATCH v4] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-08-23 10:17 UTC (permalink / raw)
To: Alexander Graf, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <53F5B77D.3000007@suse.de>
On Thursday 21 August 2014 02:40 PM, Alexander Graf wrote:
>
>
> On 20.08.14 07:52, Madhavan Srinivasan wrote:
>> This patch adds kernel side support for software breakpoint.
>> Design is that, by using an illegal instruction, we trap to hypervisor
>> via Emulation Assistance interrupt, where we check for the illegal instruction
>> and accordingly we return to Host or Guest. Patch also adds support for
>> software breakpoint in PR KVM.
>>
>> Changes v3->v4:
>> Made changes to code comments and removed #define of zero opcode
>> Added a new function to handle the debug instruction emulation in book3s_hv
>> Rebased the code to latest upstream source.
>>
>> Changes v2->v3:
>> Changed the debug instructions. Using the all zero opcode in the instruction word
>> as illegal instruction as mentioned in Power ISA instead of ABS
>> Removed reg updated in emulation assist and added a call to
>> kvmppc_emulate_instruction for reg update.
>>
>> Changes v1->v2:
>>
>> Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also share it.
>> Added code to use KVM get one reg infrastructure to get debug opcode.
>> Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
>> Made changes to commit message.
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/kvm_book3s.h | 7 +++++++
>> arch/powerpc/kvm/book3s.c | 3 ++-
>> arch/powerpc/kvm/book3s_hv.c | 32 ++++++++++++++++++++++++++++++--
>> arch/powerpc/kvm/book3s_pr.c | 3 +++
>> arch/powerpc/kvm/emulate.c | 11 +++++++++++
>> 5 files changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
>> index 6acf0c2..a1944f8 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -24,6 +24,13 @@
>> #include <linux/kvm_host.h>
>> #include <asm/kvm_book3s_asm.h>
>>
>> +/*
>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software Breakpoint.
>> + * Based on PowerISA v2.07, Instruction with primary opcode 0 will be treated as illegal
>> + * instruction.
>> + */
>> +#define KVMPPC_INST_BOOK3S_DEBUG 0x00dddd00
>
> Please change the BookE version of this as well, put the define in a
> common header and use a non book specific name.
>
I first wanted to get the server side in and then take up this, but i
can do it with this, just concerned incase of booke testing :(
>> +
>> struct kvmppc_bat {
>> u64 raw;
>> u32 bepi;
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index dd03f6b..00e9c9f 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>> struct kvm_guest_debug *dbg)
>> {
>> - return -EINVAL;
>> + vcpu->guest_debug = dbg->control;
>> + return 0;
>> }
>>
>> void kvmppc_decrementer_func(unsigned long data)
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 27cced9..0a92e45 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -725,6 +725,14 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>> return kvmppc_hcall_impl_hv_realmode(cmd);
>> }
>>
>> +static int kvmppc_emulate_debug_instruction_hv(struct kvm_run *run,
>> + struct kvm_vcpu *vcpu)
>> +{
>> + run->exit_reason = KVM_EXIT_DEBUG;
>> + run->debug.arch.address = kvmppc_get_pc(vcpu);
>> + return 0;
>> +}
>> +
>> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>> struct task_struct *tsk)
>> {
>> @@ -811,9 +819,26 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>> * we don't emulate any guest instructions at this stage.
>
> This comment is no longer true, it should get changed.
>
Will change it.
>> */
>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> - r = RESUME_GUEST;
>> + {
>> + u32 last_inst;
>> + if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
>> + EMULATE_DONE) {
>
> Please only do this if debugging is active.
>
Ok sure. But i will add in the else part of the check. Since we do the
instruction check there.
>> + /*
>> + * Fetch failed, so return to guest and
>> + * try executing it again.
>> + */
>> + r = RESUME_GUEST;
>> + } else {
>> + if (last_inst == KVMPPC_INST_BOOK3S_DEBUG) {
>> + kvmppc_emulate_debug_instruction_hv(run, vcpu);
>> + r = RESUME_HOST;
>> + } else {
>> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> + r = RESUME_GUEST;
>> + }
>> + }
>> break;
>> + }
>> /*
>> * This occurs if the guest (kernel or userspace), does something that
>> * is prohibited by HFSCR. We just generate a program interrupt to
>> @@ -922,6 +947,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
>> long int i;
>>
>> switch (id) {
>> + case KVM_REG_PPC_DEBUG_INST:
>> + *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
>> + break;
>> case KVM_REG_PPC_HIOR:
>> *val = get_reg_val(id, 0);
>> break;
>> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
>> index faffb27..cd2a39d 100644
>> --- a/arch/powerpc/kvm/book3s_pr.c
>> +++ b/arch/powerpc/kvm/book3s_pr.c
>> @@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id,
>> int r = 0;
>>
>> switch (id) {
>> + case KVM_REG_PPC_DEBUG_INST:
>> + *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
>> + break;
>> case KVM_REG_PPC_HIOR:
>> *val = get_reg_val(id, to_book3s(vcpu)->hior);
>> break;
>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
>> index e96b50d..68f38b2 100644
>> --- a/arch/powerpc/kvm/emulate.c
>> +++ b/arch/powerpc/kvm/emulate.c
>> @@ -274,6 +274,17 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
>> }
>> break;
>>
>> + case 0:
>> + /*
>> + * Instruction with primary opcode 0. Based on PowerISA
>> + * these are illegal instructions.
>> + */
>> + run->exit_reason = KVM_EXIT_DEBUG;
>
> This is wrong. We need to check that the instruction is 0x00dddd00. Any
> other op0 instruction should trigger an invalid instruction trap.
>
Ok.
Thanks for review
Regards
Maddy
>
> Alex
>
>> + run->debug.arch.address = kvmppc_get_pc(vcpu);
>> + emulated = EMULATE_EXIT_USER;
>> + advance = 0;
>> + break;
>> +
>> default:
>> emulated = EMULATE_FAIL;
>> }
>>
>
^ permalink raw reply
* Re: [PATCH v4] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-08-23 12:24 UTC (permalink / raw)
To: Madhavan Srinivasan
Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, paulus@samba.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <53F86A4A.6080303@linux.vnet.ibm.com>
> Am 23.08.2014 um 12:17 schrieb Madhavan Srinivasan <maddy@linux.vnet.ibm.c=
om>:
>=20
>> On Thursday 21 August 2014 02:40 PM, Alexander Graf wrote:
>>=20
>>=20
>>> On 20.08.14 07:52, Madhavan Srinivasan wrote:
>>> This patch adds kernel side support for software breakpoint.
>>> Design is that, by using an illegal instruction, we trap to hypervisor
>>> via Emulation Assistance interrupt, where we check for the illegal instr=
uction
>>> and accordingly we return to Host or Guest. Patch also adds support for
>>> software breakpoint in PR KVM.
>>>=20
>>> Changes v3->v4:
>>> Made changes to code comments and removed #define of zero opcode
>>> Added a new function to handle the debug instruction emulation in book3s=
_hv
>>> Rebased the code to latest upstream source.
>>>=20
>>> Changes v2->v3:
>>> Changed the debug instructions. Using the all zero opcode in the instruc=
tion word
>>> as illegal instruction as mentioned in Power ISA instead of ABS
>>> Removed reg updated in emulation assist and added a call to
>>> kvmppc_emulate_instruction for reg update.
>>>=20
>>> Changes v1->v2:
>>>=20
>>> Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can al=
so share it.
>>> Added code to use KVM get one reg infrastructure to get debug opcode.
>>> Updated emulate.c to include emulation of debug instruction incase of PR=
_KVM.
>>> Made changes to commit message.
>>>=20
>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/include/asm/kvm_book3s.h | 7 +++++++
>>> arch/powerpc/kvm/book3s.c | 3 ++-
>>> arch/powerpc/kvm/book3s_hv.c | 32 +++++++++++++++++++++++++++++=
+--
>>> arch/powerpc/kvm/book3s_pr.c | 3 +++
>>> arch/powerpc/kvm/emulate.c | 11 +++++++++++
>>> 5 files changed, 53 insertions(+), 3 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/includ=
e/asm/kvm_book3s.h
>>> index 6acf0c2..a1944f8 100644
>>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>>> @@ -24,6 +24,13 @@
>>> #include <linux/kvm_host.h>
>>> #include <asm/kvm_book3s_asm.h>
>>>=20
>>> +/*
>>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Softwar=
e Breakpoint.
>>> + * Based on PowerISA v2.07, Instruction with primary opcode 0 will be t=
reated as illegal
>>> + * instruction.
>>> + */
>>> +#define KVMPPC_INST_BOOK3S_DEBUG 0x00dddd00
>>=20
>> Please change the BookE version of this as well, put the define in a
>> common header and use a non book specific name.
>=20
> I first wanted to get the server side in and then take up this, but i
> can do it with this, just concerned incase of booke testing :(
I can test, no problem. You can at least do compile tests on your side alrea=
dy ;)
>=20
>>> +
>>> struct kvmppc_bat {
>>> u64 raw;
>>> u32 bepi;
>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>> index dd03f6b..00e9c9f 100644
>>> --- a/arch/powerpc/kvm/book3s.c
>>> +++ b/arch/powerpc/kvm/book3s.c
>>> @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *v=
cpu,
>>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> struct kvm_guest_debug *dbg)
>>> {
>>> - return -EINVAL;
>>> + vcpu->guest_debug =3D dbg->control;
>>> + return 0;
>>> }
>>>=20
>>> void kvmppc_decrementer_func(unsigned long data)
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c=
>>> index 27cced9..0a92e45 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -725,6 +725,14 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>>> return kvmppc_hcall_impl_hv_realmode(cmd);
>>> }
>>>=20
>>> +static int kvmppc_emulate_debug_instruction_hv(struct kvm_run *run,
>>> + struct kvm_vcpu *vcpu)
>>> +{
>>> + run->exit_reason =3D KVM_EXIT_DEBUG;
>>> + run->debug.arch.address =3D kvmppc_get_pc(vcpu);
>>> + return 0;
>>> +}
>>> +
>>> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *v=
cpu,
>>> struct task_struct *tsk)
>>> {
>>> @@ -811,9 +819,26 @@ static int kvmppc_handle_exit_hv(struct kvm_run *ru=
n, struct kvm_vcpu *vcpu,
>>> * we don't emulate any guest instructions at this stage.
>>=20
>> This comment is no longer true, it should get changed.
>=20
> Will change it.
>=20
>>> */
>>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>>> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>>> - r =3D RESUME_GUEST;
>>> + {
>>> + u32 last_inst;
>>> + if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=3D
>>> + EMULATE_DONE) {
>>=20
>> Please only do this if debugging is active.
>=20
> Ok sure. But i will add in the else part of the check. Since we do the
> instruction check there.
We should only fetch the instruction if sw breakpoint debugging is active.
Alex
^ permalink raw reply
* Re: [PATCH v3] arch: Kconfig: Let all architectures set endian explicitly
From: Chen Gang @ 2014-08-24 8:38 UTC (permalink / raw)
To: Ralf Baechle
Cc: linux-m32r-ja, linux-mips, linux-m68k, a-jacquiot,
catalin.marinas, linux, will.deacon, dhowells, jcmvbkbc, paulus,
hpa, sparclinux, linux-hexagon, linux-s390, gxt, Jean Delvare,
egtvedt, jonas, Liqin Chen, jesper.nilsson, linux-am33-list,
linux, linux-c6x-dev, David S. Miller, linux-xtensa, takata, x86,
jejb, torvalds, linux-sh, mingo, Geert Uytterhoeven, Lennox Wu,
msalter, mattst88, hskinnemoen, fenghua.yu, james.hogan,
Arnd Bergmann, realmz6, heiko.carstens, jdike, linux-alpha,
adi-buildroot-devel, cmetcalf, starvik, linux-m32r, ink,
user-mode-linux-user, tglx, linux-metag, linux-arm-kernel, rth,
chris, Michal Simek, tony.luck, linux-ia64, linux-parisc,
linux-cris-kernel, vgupta, linux-kernel@vger.kernel.org, rkuo,
user-mode-linux-devel, Richard Weinberger, Martin Schwidefsky,
linux390, akpm, yasutake.koichi, linuxppc-dev, deller
In-Reply-To: <53EDCC60.5040608@gmail.com>
Hello Maintainers:
Is this patch OK? If it pass basic checking, please let me know, and I
shall try to make another related patch for KBuild (I can do nothing
related with Kbuild, before get confirmation for this patch).
Thanks.
On 8/15/14 17:01, Chen Gang wrote:
>
>
> On 8/15/14 6:14, Chen Gang wrote:
>> On 08/15/2014 02:04 AM, Ralf Baechle wrote:
>>>
>>
>> OK, thanks, I assumes when support both endian, the default choice is
>> CPU_BIG_ENDIAN, although no default value for choice (originally, I did
>> worry about it).
>>
>>> So I think you can just drop the MIPS segment from your patch.
>>>
>>
>> If what I assumes is correct, what you said sounds reasonable to me.
>>
>>
>
> So for me, it is harmless to add CPU_*_ENDIAN explicitly, and can let
> other members don't need think of.
>
> By the way, for sh, it is almost the same case, except it contents the
> default value, for me, it is clear enough, so I skip sh architecture in
> this patch.
>
>
> Thanks
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Drop unnecessary continue
From: Michael Ellerman @ 2014-08-25 4:16 UTC (permalink / raw)
To: Robert Jennings
Cc: linux-kernel, Julia Lawall, Himangi Saraogi, Brian King,
Paul Mackerras, linuxppc-dev
In-Reply-To: <53F61586.3000107@gmail.com>
On Thu, 2014-08-21 at 10:51 -0500, Robert Jennings wrote:
> On 08/20/2014 11:41 PM, Michael Ellerman wrote:
> > On Wed, 2014-08-13 at 14:48 +0530, Himangi Saraogi wrote:
> >> Continue is not needed at the bottom of a loop.
> >
> > True.
> >
> > I wonder though, is the code trying to continue to the outer loop?
> > I stared at it for a minute but it wasn't obvious.
> >
> > I wonder if Robert still remembers?
>
> I don't recall what the intent was here. Can't believe that it's been
> almost 5 years since I wrote this. I wish I had left a few more
> comments in the code for me to go on.
>
> Obviously the continue should be removed since it's not doing
> anything. I don't believe that we'd want a continue statement in
> there to get outer loop. That would change the current cmm_page_array
> pointer (pa_curr) to the next in the list after it may have just been
> reassigned to pa_last->next.
>
> It may be the case that an earlier version of the code had statements
> in the inner loop after that continue that I wanted to skip, or I just
> did something silly.
OK, thanks for looking at it. I came to a similar conclusion, but good to have
your review as well.
The CMM regression test suite will catch us if we get it wrong anyway.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: edac: Fix build error
From: Michael Ellerman @ 2014-08-25 5:29 UTC (permalink / raw)
To: Pranith Kumar
Cc: open list, Borislav Petkov, Doug Thompson, Paul Mackerras,
Andrew Morton, open list:EDAC-CORE,
open list:LINUX FOR POWERPC..., Mauro Carvalho Chehab
In-Reply-To: <1408673991-17892-1-git-send-email-bobby.prani@gmail.com>
On Thu, 2014-08-21 at 22:19 -0400, Pranith Kumar wrote:
> Fix the following build error:
>
> drivers/edac/ppc4xx_edac.c: In function 'mfsdram':
> drivers/edac/ppc4xx_edac.c:249: error: implicit declaration of function
> '__mfdcri'
> drivers/edac/ppc4xx_edac.c: In function 'mtsdram':
> drivers/edac/ppc4xx_edac.c:266: error: implicit declaration of function
> '__mtdcri'
> drivers/edac/ppc4xx_edac.c:269: warning: 'return' with a value, in function
> returning void
> drivers/edac/ppc4xx_edac.c: In function 'ppc4xx_edac_init_csrows':
> drivers/edac/ppc4xx_edac.c:924: warning: initialization from incompatible
> pointer type
> drivers/edac/ppc4xx_edac.c:977: error: request for member 'dimm' in something
> not a structure or union
> drivers/edac/ppc4xx_edac.c: In function 'ppc4xx_edac_map_dcrs':
> drivers/edac/ppc4xx_edac.c:1209: warning: passing argument 1 of 'dcr_map_mmio'
> discards qualifiers from pointer target type
>
> This driver depends on PPC_DCR_NATIVE to be set for the relevant headers to be
> included. Also if PPC_DCR_MMIO=n the build fails. So make PPC_DCR depend on both
> these options.
Um, NAK I think. The whole point is that some platforms implement DCR natively
and some via MMIO, and that's meant to be hidden by the DCR API.
If the driver is directly calling DCR native routines then it should depend on
that.
cheers
^ permalink raw reply
* Re: [RFC PATCH powerpc] Fix warning reported by verify_cpu_node_mapping()
From: Li Zhong @ 2014-08-25 6:01 UTC (permalink / raw)
To: Nishanth Aravamudan; +Cc: Michael Ellerman, PowerPC email list, Paul Mackerras
In-Reply-To: <20140822220406.GA20951@linux.vnet.ibm.com>
On 五, 2014-08-22 at 15:04 -0700, Nishanth Aravamudan wrote:
> On 22.08.2014 [10:12:56 +0800], Li Zhong wrote:
> > On ???, 2014-08-21 at 08:45 -0700, Nishanth Aravamudan wrote:
> > > On 21.08.2014 [16:14:02 +0800], Li Zhong wrote:
> > > > With commit 2fabf084b, 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 could see something like following reported:
> > > > [ 0.000000] CPU thread siblings 1/2/3 and 0 don't belong to the same
> > > > node!
> > >
> > > You mean you did see this, right? (as opposed to "could" based upon code
> > > inspection or something)
> >
> > Yes, I did see the warnings. Seems I didn't express it precisely in
> > English ...
> >
> > >
> > > >
> > > > 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()?
> > > >
> > > > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> > >
> > > Acked-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> >
> > Thank you for the review,
> >
> > Zhong
> >
> > >
> > > > ---
> > > > 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);
> > >
> > > This is a good change, thanks for catching it. I must have glossed over
> > > those messages in my testing, my apologies!
>
> Actually, thinking about this more, do you think it makes more sense to
> do:
>
> for_each_present_cpu(cpu) in this loop? That is, at boot, ensure all
> present (but possibly offline) CPUs have their NUMA mapping set up. CPUs
> that aren't present (but are possible) might trigger other warnings,
> right? (e.g., the WARN_ON(1) in numa_setup_cpu)
After reading the code that set up the cpu masks, I think you are right.
I will send a new version with this fixed.
Thanks, Zhong
>
> -Nish
^ permalink raw reply
* Re: [PATCH 2/2] fsl_ifc: Support all 8 IFC chip selects
From: Prabhakar Kushwaha @ 2014-08-25 6:28 UTC (permalink / raw)
To: Scott Wood; +Cc: Greg Kroah-Hartman, Aaron Sierra, linuxppc-dev, Arnd Bergmann
In-Reply-To: <1408729900.6510.2.camel@snotra.buserror.net>
On 8/22/2014 11:21 PM, Scott Wood wrote:
> On Fri, 2014-08-22 at 20:07 +0530, Prabhakar Kushwaha wrote:
>> Sorry Scott for late reply,
>>
>> Please find my reply in-lined
>>
>>
>> On 8/21/2014 4:51 AM, Scott Wood wrote:
>>> On Wed, 2014-08-20 at 09:05 +0530, Prabhakar Kushwaha wrote:
>>>> On 8/20/2014 5:38 AM, Scott Wood wrote:
>>>>> On Fri, 2014-08-15 at 16:07 -0500, Aaron Sierra wrote:
>>>>>> Freescale's QorIQ T Series processors support 8 IFC chip selects
>>>>>> within a memory map backward compatible with previous P Series
>>>>>> processors which supported only 4 chip selects.
>>>>>>
>>>>>> Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
>>>>>> ---
>>>>>> include/linux/fsl_ifc.h | 10 +++++-----
>>>>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
>>>>>> index 84d60cb..62762ff 100644
>>>>>> --- a/include/linux/fsl_ifc.h
>>>>>> +++ b/include/linux/fsl_ifc.h
>>>>>> @@ -29,7 +29,7 @@
>>>>>> #include <linux/of_platform.h>
>>>>>> #include <linux/interrupt.h>
>>>>>>
>>>>>> -#define FSL_IFC_BANK_COUNT 4
>>>>>> +#define FSL_IFC_BANK_COUNT 8
>>>>> First please modify fsl_ifc_nand.c to limit itself to the number of
>>>>> banks it dynamically determines are present based on the IFC version.
>>>>>
>>>>>
>>>> Number of available bank/chip select are defined by SoC and it is
>>>> independent of SoC.
>>> Do you mean defined by the SoC and independent of the IFC version?
>> IFC v 1.0.0 supports 4 Chip Select.
>> IFC v 1.1.0 onwards, IFC supports 8 chip select.
>>
>> But SoC finally defines number of chip select coming out of SoC. Like
>> LS1021A with IFC ver 1.4.0 have only 7 Chip Select.
> What matters here is whether the registers are implemented, not whether
> a chip select is pinned out -- so use the IFC version.
I checked with IFC IP team " There is no side-effect of reading CS-8
register if only 7 CS exposed by SoC"
IFC version can be used.
Regards,
Prabhakar
^ permalink raw reply
* [PATCH v2] powerpc: Fix warning reported by verify_cpu_node_mapping()
From: Li Zhong @ 2014-08-25 7:22 UTC (permalink / raw)
To: Nishanth Aravamudan; +Cc: Michael Ellerman, PowerPC email list, Paul Mackerras
In-Reply-To: <20140822220406.GA20951@linux.vnet.ibm.com>
With commit 2fabf084b, 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().
Also, as Nish suggested, here it's better to use present cpu mask
instead of possible mask to avoid warning in numa_setup_cpu().
Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index d7737a5..3a9061e 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1127,9 +1127,8 @@ void __init do_init_bootmem(void)
* even before we online them, so that we can use cpu_to_{node,mem}
* 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);
+ for_each_present_cpu(cpu) {
+ numa_setup_cpu((unsigned long)cpu);
}
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox