Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling

2013-05-24 Thread David Gibson
On Wed, May 22, 2013 at 04:06:57PM -0500, Scott Wood wrote:
> On 05/20/2013 10:06:46 PM, Alexey Kardashevskiy wrote:
> >diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> >index 8465c2a..da6bf61 100644
> >--- a/arch/powerpc/kvm/powerpc.c
> >@@ -396,6 +396,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> >+++ b/arch/powerpc/kvm/powerpc.c
> > break;
> > #endif
> > case KVM_CAP_SPAPR_MULTITCE:
> >+case KVM_CAP_SPAPR_TCE_IOMMU:
> > r = 1;
> > break;
> > default:
> 
> Don't advertise SPAPR capabilities if it's not book3s -- and
> probably there's some additional limitation that would be
> appropriate.

So, in the case of MULTITCE, that's not quite right.  PR KVM can
emulate a PAPR system on a BookE machine, and there's no reason not to
allow TCE acceleration as well.  We can't make it dependent on PAPR
mode being selected, because that's enabled per-vcpu, whereas these
capabilities are queried on the VM before the vcpus are created.

CAP_SPAPR_TCE_IOMMU should be dependent on the presence of suitable
host side hardware (i.e. a PAPR style IOMMU), though.

> 
> >@@ -1025,6 +1026,17 @@ long kvm_arch_vm_ioctl(struct file *filp,
> > r = kvm_vm_ioctl_create_spapr_tce(kvm, &create_tce);
> > goto out;
> > }
> >+case KVM_CREATE_SPAPR_TCE_IOMMU: {
> >+struct kvm_create_spapr_tce_iommu create_tce_iommu;
> >+struct kvm *kvm = filp->private_data;
> >+
> >+r = -EFAULT;
> >+if (copy_from_user(&create_tce_iommu, argp,
> >+sizeof(create_tce_iommu)))
> >+goto out;
> >+r = kvm_vm_ioctl_create_spapr_tce_iommu(kvm,
> >&create_tce_iommu);
> >+goto out;
> >+}
> > #endif /* CONFIG_PPC_BOOK3S_64 */
> >
> > #ifdef CONFIG_KVM_BOOK3S_64_HV
> >diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> >index 5a2afda..450c82a 100644
> >--- a/include/uapi/linux/kvm.h
> >+++ b/include/uapi/linux/kvm.h
> >@@ -667,6 +667,7 @@ struct kvm_ppc_smmu_info {
> > #define KVM_CAP_PPC_RTAS 91
> > #define KVM_CAP_IRQ_XICS 92
> > #define KVM_CAP_SPAPR_MULTITCE (0x11 + 89)
> >+#define KVM_CAP_SPAPR_TCE_IOMMU (0x11 + 90)
> 
> Hmm...

Ah, yeah, that needs to be fixed.  Those were interim numbers so that
we didn't have to keep changing our internal trees as new upstream
ioctls got added to the list.  We need to get a proper number for the
merge, though.

> >@@ -939,6 +940,9 @@ struct kvm_s390_ucas_mapping {
> > #define KVM_GET_DEVICE_ATTR   _IOW(KVMIO,  0xe2, struct
> >kvm_device_attr)
> > #define KVM_HAS_DEVICE_ATTR   _IOW(KVMIO,  0xe3, struct
> >kvm_device_attr)
> >
> >+/* ioctl for SPAPR TCE IOMMU */
> >+#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO,  0xe4, struct
> >kvm_create_spapr_tce_iommu)
> 
> Shouldn't this go under the vm ioctl section?
> 
> -Scott
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH RESEND v2 2/2] serial/mpc52xx_uart: add MPC5125 PSC support

2013-05-24 Thread Anatolij Gustschin
From: Matteo Facchinetti 

Add MPC5125 PSC register layout structure, MPC5125 specific
psc_ops function set and the compatible string.

Signed-off-by: Vladimir Ermakov 
Signed-off-by: Matteo Facchinetti 
Signed-off-by: Anatolij Gustschin 
---
Changes in v2:
 - split into two patches to simplify review
 - minor coding style changes 
 - revise commit log

 arch/powerpc/include/asm/mpc52xx_psc.h |   49 +++
 drivers/tty/serial/mpc52xx_uart.c  |  241 
 2 files changed, 290 insertions(+)

diff --git a/arch/powerpc/include/asm/mpc52xx_psc.h 
b/arch/powerpc/include/asm/mpc52xx_psc.h
index 2966df6..d0ece25 100644
--- a/arch/powerpc/include/asm/mpc52xx_psc.h
+++ b/arch/powerpc/include/asm/mpc52xx_psc.h
@@ -299,4 +299,53 @@ struct mpc512x_psc_fifo {
 #define rxdata_32 rxdata.rxdata_32
 };
 
+struct mpc5125_psc {
+   u8  mr1;/* PSC + 0x00 */
+   u8  reserved0[3];
+   u8  mr2;/* PSC + 0x04 */
+   u8  reserved1[3];
+   struct {
+   u16 status; /* PSC + 0x08 */
+   u8  reserved2[2];
+   u8  clock_select;   /* PSC + 0x0c */
+   u8  reserved3[3];
+   } sr_csr;
+   u8  command;/* PSC + 0x10 */
+   u8  reserved4[3];
+   union { /* PSC + 0x14 */
+   u8  buffer_8;
+   u16 buffer_16;
+   u32 buffer_32;
+   } buffer;
+   struct {
+   u8  ipcr;   /* PSC + 0x18 */
+   u8  reserved5[3];
+   u8  acr;/* PSC + 0x1c */
+   u8  reserved6[3];
+   } ipcr_acr;
+   struct {
+   u16 isr;/* PSC + 0x20 */
+   u8  reserved7[2];
+   u16 imr;/* PSC + 0x24 */
+   u8  reserved8[2];
+   } isr_imr;
+   u8  ctur;   /* PSC + 0x28 */
+   u8  reserved9[3];
+   u8  ctlr;   /* PSC + 0x2c */
+   u8  reserved10[3];
+   u32 ccr;/* PSC + 0x30 */
+   u32 ac97slots;  /* PSC + 0x34 */
+   u32 ac97cmd;/* PSC + 0x38 */
+   u32 ac97data;   /* PSC + 0x3c */
+   u8  reserved11[4];
+   u8  ip; /* PSC + 0x44 */
+   u8  reserved12[3];
+   u8  op1;/* PSC + 0x48 */
+   u8  reserved13[3];
+   u8  op0;/* PSC + 0x4c */
+   u8  reserved14[3];
+   u32 sicr;   /* PSC + 0x50 */
+   u8  reserved15[4];  /* make eq. sizeof(mpc52xx_psc) */
+};
+
 #endif  /* __ASM_MPC52xx_PSC_H__ */
diff --git a/drivers/tty/serial/mpc52xx_uart.c 
b/drivers/tty/serial/mpc52xx_uart.c
index 5aa87ac..9c3eab5 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -658,6 +658,246 @@ static void mpc512x_psc_get_irq(struct uart_port *port, 
struct device_node *np)
port->irqflags = IRQF_SHARED;
port->irq = psc_fifoc_irq;
 }
+#endif
+
+#ifdef CONFIG_PPC_MPC512x
+
+#define PSC_5125(port) ((struct mpc5125_psc __iomem *)((port)->membase))
+#define FIFO_5125(port) ((struct mpc512x_psc_fifo __iomem *)(PSC_5125(port)+1))
+
+static void mpc5125_psc_fifo_init(struct uart_port *port)
+{
+   /* /32 prescaler */
+   out_8(&PSC_5125(port)->mpc52xx_psc_clock_select, 0xdd);
+
+   out_be32(&FIFO_5125(port)->txcmd, MPC512x_PSC_FIFO_RESET_SLICE);
+   out_be32(&FIFO_5125(port)->txcmd, MPC512x_PSC_FIFO_ENABLE_SLICE);
+   out_be32(&FIFO_5125(port)->txalarm, 1);
+   out_be32(&FIFO_5125(port)->tximr, 0);
+
+   out_be32(&FIFO_5125(port)->rxcmd, MPC512x_PSC_FIFO_RESET_SLICE);
+   out_be32(&FIFO_5125(port)->rxcmd, MPC512x_PSC_FIFO_ENABLE_SLICE);
+   out_be32(&FIFO_5125(port)->rxalarm, 1);
+   out_be32(&FIFO_5125(port)->rximr, 0);
+
+   out_be32(&FIFO_5125(port)->tximr, MPC512x_PSC_FIFO_ALARM);
+   out_be32(&FIFO_5125(port)->rximr, MPC512x_PSC_FIFO_ALARM);
+}
+
+static int mpc5125_psc_raw_rx_rdy(struct uart_port *port)
+{
+   return !(in_be32(&FIFO_5125(port)->rxsr) & MPC512x_PSC_FIFO_EMPTY);
+}
+
+static int mpc5125_psc_raw_tx_rdy(struct uart_port *port)
+{
+   return !(in_be32(&FIFO_5125(port)->txsr) & MPC512x_PSC_FIFO_FULL);
+}
+
+static int mpc5125_psc_rx_rdy(struct uart_port *port)
+{
+   return in_be32(&FIFO_5125(port)->rxsr) &
+  in_be32(&FIFO_5125(port)->rximr) & MPC512x_PSC_FIFO_ALARM;
+}
+
+static int mpc5125_psc_tx_rdy

[PATCH RESEND v2 1/2] serial/mpc52xx_uart: prepare for adding MPC5125 PSC UART support

2013-05-24 Thread Anatolij Gustschin
From: Matteo Facchinetti 

MPC5125 PSC controller has different register layout than MPC5121.
To support MPC5125 PSC in this driver we have to provide further
psc_ops functions for SoC specific register accesses.

Add new register access functions to the psc_ops structure and
provide MPC52xx and MPC512x specific implementation for them.
Then replace remaining direct register accesses in the driver by
appropriate psc_ops function calls. The subsequent patch can now
add MPC5125 specific set of psc_ops functions.

Signed-off-by: Vladimir Ermakov 
Signed-off-by: Matteo Facchinetti 
Signed-off-by: Anatolij Gustschin 
---

Changes in v2:
 - split into two patches to simplify review
 - minor coding style changes
 - revise commit log

 drivers/tty/serial/mpc52xx_uart.c |  161 +++--
 1 file changed, 119 insertions(+), 42 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c 
b/drivers/tty/serial/mpc52xx_uart.c
index 018bad9..5aa87ac 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -122,6 +122,15 @@ struct psc_ops {
void(*fifoc_uninit)(void);
void(*get_irq)(struct uart_port *, struct device_node *);
irqreturn_t (*handle_irq)(struct uart_port *port);
+   u16 (*get_status)(struct uart_port *port);
+   u8  (*get_ipcr)(struct uart_port *port);
+   void(*command)(struct uart_port *port, u8 cmd);
+   void(*set_mode)(struct uart_port *port, u8 mr1, u8 mr2);
+   void(*set_rts)(struct uart_port *port, int state);
+   void(*enable_ms)(struct uart_port *port);
+   void(*set_sicr)(struct uart_port *port, u32 val);
+   void(*set_imr)(struct uart_port *port, u16 val);
+   u8  (*get_mr1)(struct uart_port *port);
 };
 
 /* setting the prescaler and divisor reg is common for all chips */
@@ -134,6 +143,65 @@ static inline void mpc52xx_set_divisor(struct mpc52xx_psc 
__iomem *psc,
out_8(&psc->ctlr, divisor & 0xff);
 }
 
+static u16 mpc52xx_psc_get_status(struct uart_port *port)
+{
+   return in_be16(&PSC(port)->mpc52xx_psc_status);
+}
+
+static u8 mpc52xx_psc_get_ipcr(struct uart_port *port)
+{
+   return in_8(&PSC(port)->mpc52xx_psc_ipcr);
+}
+
+static void mpc52xx_psc_command(struct uart_port *port, u8 cmd)
+{
+   out_8(&PSC(port)->command, cmd);
+}
+
+static void mpc52xx_psc_set_mode(struct uart_port *port, u8 mr1, u8 mr2)
+{
+   out_8(&PSC(port)->command, MPC52xx_PSC_SEL_MODE_REG_1);
+   out_8(&PSC(port)->mode, mr1);
+   out_8(&PSC(port)->mode, mr2);
+}
+
+static void mpc52xx_psc_set_rts(struct uart_port *port, int state)
+{
+   if (state)
+   out_8(&PSC(port)->op1, MPC52xx_PSC_OP_RTS);
+   else
+   out_8(&PSC(port)->op0, MPC52xx_PSC_OP_RTS);
+}
+
+static void mpc52xx_psc_enable_ms(struct uart_port *port)
+{
+   struct mpc52xx_psc __iomem *psc = PSC(port);
+
+   /* clear D_*-bits by reading them */
+   in_8(&psc->mpc52xx_psc_ipcr);
+   /* enable CTS and DCD as IPC interrupts */
+   out_8(&psc->mpc52xx_psc_acr, MPC52xx_PSC_IEC_CTS | MPC52xx_PSC_IEC_DCD);
+
+   port->read_status_mask |= MPC52xx_PSC_IMR_IPC;
+   out_be16(&psc->mpc52xx_psc_imr, port->read_status_mask);
+}
+
+static void mpc52xx_psc_set_sicr(struct uart_port *port, u32 val)
+{
+   out_be32(&PSC(port)->sicr, val);
+}
+
+static void mpc52xx_psc_set_imr(struct uart_port *port, u16 val)
+{
+   out_be16(&PSC(port)->mpc52xx_psc_imr, val);
+}
+
+static u8 mpc52xx_psc_get_mr1(struct uart_port *port)
+{
+   out_8(&PSC(port)->command, MPC52xx_PSC_SEL_MODE_REG_1);
+   return in_8(&PSC(port)->mode);
+}
+
 #ifdef CONFIG_PPC_MPC52xx
 #define FIFO_52xx(port) ((struct mpc52xx_psc_fifo __iomem *)(PSC(port)+1))
 static void mpc52xx_psc_fifo_init(struct uart_port *port)
@@ -304,6 +372,15 @@ static struct psc_ops mpc52xx_psc_ops = {
.set_baudrate = mpc5200_psc_set_baudrate,
.get_irq = mpc52xx_psc_get_irq,
.handle_irq = mpc52xx_psc_handle_irq,
+   .get_status = mpc52xx_psc_get_status,
+   .get_ipcr = mpc52xx_psc_get_ipcr,
+   .command = mpc52xx_psc_command,
+   .set_mode = mpc52xx_psc_set_mode,
+   .set_rts = mpc52xx_psc_set_rts,
+   .enable_ms = mpc52xx_psc_enable_ms,
+   .set_sicr = mpc52xx_psc_set_sicr,
+   .set_imr = mpc52xx_psc_set_imr,
+   .get_mr1 = mpc52xx_psc_get_mr1,
 };
 
 static struct psc_ops mpc5200b_psc_ops = {
@@ -325,6 +402,15 @@ static struct psc_ops mpc5200b_psc_ops = {
.set_baudrate = mpc5200b_psc_set_baudrate,
.get_irq = mpc52xx_psc_get_irq,
.handle_irq = mpc52xx_psc_handle_irq,
+   .get_status = mpc52xx_psc_get_status,
+   .get_ipcr = mpc52xx_psc_get_ipcr,
+   .command = mpc52xx_psc_command,
+   .set_mode = mpc52xx_psc_set_mode,
+   .set_rts = mpc52xx_psc_set_rts,
+   .enable_ms = mpc

Re: [PATCH v2 1/2] serial/mpc52xx_uart: prepare for adding MPC5125 PSC UART support

2013-05-24 Thread Greg Kroah-Hartman
On Fri, May 24, 2013 at 07:49:12PM +0200, Anatolij Gustschin wrote:
> On Wed, 17 Apr 2013 23:21:41 +0200
> Anatolij Gustschin  wrote:
> 
> > From: Matteo Facchinetti 
> > 
> > MPC5125 PSC controller has different register layout than MPC5121.
> > To support MPC5125 PSC in this driver we have to provide further
> > psc_ops functions for SoC specific register accesses.
> > 
> > Add new register access functions to the psc_ops structure and
> > provide MPC52xx and MPC512x specific implementation for them.
> > Then replace remaining direct register accesses in the driver by
> > appropriate psc_ops function calls. The subsequent patch can now
> > add MPC5125 specific set of psc_ops functions.
> > 
> > Signed-off-by: Vladimir Ermakov 
> > Signed-off-by: Matteo Facchinetti 
> > Signed-off-by: Anatolij Gustschin 
> > ---
> > 
> > Greg, with your Acked-by I can push these patches to my mpc5xxx tree.
> > But it is fine with me if you prefer to apply them to tty tree.
> 
> ping ...

Sorry, I somehow lost this, so I can't see the original patch at all.
Care to resend it?

thanks,

greg k-h
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 1/2] serial/mpc52xx_uart: prepare for adding MPC5125 PSC UART support

2013-05-24 Thread Anatolij Gustschin
On Wed, 17 Apr 2013 23:21:41 +0200
Anatolij Gustschin  wrote:

> From: Matteo Facchinetti 
> 
> MPC5125 PSC controller has different register layout than MPC5121.
> To support MPC5125 PSC in this driver we have to provide further
> psc_ops functions for SoC specific register accesses.
> 
> Add new register access functions to the psc_ops structure and
> provide MPC52xx and MPC512x specific implementation for them.
> Then replace remaining direct register accesses in the driver by
> appropriate psc_ops function calls. The subsequent patch can now
> add MPC5125 specific set of psc_ops functions.
> 
> Signed-off-by: Vladimir Ermakov 
> Signed-off-by: Matteo Facchinetti 
> Signed-off-by: Anatolij Gustschin 
> ---
> 
> Greg, with your Acked-by I can push these patches to my mpc5xxx tree.
> But it is fine with me if you prefer to apply them to tty tree.

ping ...

Thanks,

Anatolij
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Jason Gunthorpe
On Fri, May 24, 2013 at 12:46:36PM -0400, Jason Cooper wrote:

> > Why are you not keen on this? It seems like normal device driver
> > practice, that is what the data field of of_device_id is typically
> > used for..
> 
> I'm not keen on it because we don't have a document saying "All kirkwood
> SoCs need PSC1 set to X after reset."  We know it, but have we tested
> the 6282?

I disagree. The manul is very clear how PSC1 must be set for proper
operation. Clk125BypassEn bit is used only for loopback testing, it
should never set for driver operation. Similarly PortReset must be
cleared for driver operation.

It is always safe for the driver to clear these bits, if it knows they
are available.  In fact, I would argue the driver should always clear
these bits so that the bootloader isn't relied on to do it. It doesn't
matter if the SOC errantly sets the bit or not, it can *always* be
safely cleared.

Further, I compared my 88F6282/88F6283 manual against the public
88F6180/88F619x/88F6281 spec and confirmed that the PSC1 layout is the
same.

So all of these SOC's can share a driver compatible string.

AFAICT the only reason the driver doesn't touch PSC1 today is because
the PSC1 was introduced in a later IP revision and its presence isn't
auto-detectable.

The last bit of the puzzle to get bootloader independence on kirkwood
is to encode the phy interface type (GMII/RGMII/BASE-X) in the DT so
the entire PSC1 can be set by the driver..

Jason
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Sebastian Hesselbarth

On 05/24/2013 07:13 PM, Russell King - ARM Linux wrote:

Do you really want that on ARM?  Given the fiasco with the location of
the registers, are you sure you want to place more trust in that
direction?  Does it give you a warm fuzzy feeling to know that you
might have to work out some way to patch vendor supplied bytecode?


Don't get me wrong. I want mv643xx_eth DT or even platform_data to
evolve to a fully self configured driver not depending on proper u-boot
setup at all.

But I don't want to go that road now and I wonder if it might be safer
for us (and PPC guys) if we start mv643xx_eth over from scratch one day.

For this patch set, I want a basic DT binding that works. Patching the
driver to play with kirkwood loosing the MAC and other important
registers is not my main concern here. If clearing that one bit doesn't
help for all kirkwood boards, I'd rather leave the non-gating
workaround.

mv643xx_eth not knowing DT for ARM is stalling last important bits for
Orion SoCs. I want this to go in first as with David another maintainer
is involved.

Sebastian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Russell King - ARM Linux
On Fri, May 24, 2013 at 01:01:25PM -0400, Jason Cooper wrote:
> On Fri, May 24, 2013 at 01:03:25PM +0200, Linus Walleij wrote:
> > IMO: if you want to go down that road, what you really want is not
> > ever more expressible device trees, but real open firmware,
> > or ACPI or UEFI that can interpret and run bytecode as some
> > "bios" for you. With DT coming from OF maybe this is a natural
> > progression of things, but one has to realize when we reach the
> > point where what we really want is a bios. Then your time is
> > likely better spent with Tianocore or something than with the
> > kernel.
> 
> shudder.  I like embedded systems because the *don't* have a bios.

Then you're into scenarios like I have with my laptop, where - those
of you who check the nightly build results will have noticed - one
of my serial ports doesn't always exist.  That's because the ACPI data
in the BIOS is *wrong*.  It reports that it has been enabled when it
hasn't, and the disassembled byte code is at fault here.

The fix?  God knows.  As far as I'm concerned as a user, or even as an
OS developer, it's unfixable without getting the ACPI data structures
changed, and that's not something I can do.

Do you really want that on ARM?  Given the fiasco with the location of
the registers, are you sure you want to place more trust in that
direction?  Does it give you a warm fuzzy feeling to know that you
might have to work out some way to patch vendor supplied bytecode?
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Jason Cooper
On Fri, May 24, 2013 at 06:53:15PM +0200, Andrew Lunn wrote:
> > > Why are you not keen on this? It seems like normal device driver
> > > practice, that is what the data field of of_device_id is typically
> > > used for..
> > 
> > I'm not keen on it because we don't have a document saying "All kirkwood
> > SoCs need PSC1 set to X after reset."  We know it, but have we tested
> > the 6282?
> 
> 6282 looses its MAC address, that much i know. I've no idea about
> PSC1, but if its MAC address behaviour is the same as 6281, is expect
> PSC1 is the same.

Do you have a board set up for testing you could try Sebastian's
forthcoming series on (with "marvell,kirkwood-eth")?

thx,

Jason.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Jason Cooper
On Fri, May 24, 2013 at 01:03:25PM +0200, Linus Walleij wrote:
> On Fri, May 24, 2013 at 12:40 AM, Sebastian Hesselbarth
>  wrote:
> > On 05/23/2013 08:40 PM, Jason Cooper wrote:
> 
> >> I think marvell,psc1_reset =<>; gives us the most flexibility in
> >> accurately describing the hardware.
> >
> >
> > IMHO using that is just another workaround for a broken driver. We
> > could hack the whole register setup in DT as it would still accurately
> > describe HW. Don't get me wrong, but I don't like it.
> >
> > Haven't checked how happy Linus Walleij is about pinctrl drivers with
> > reg values hacked in lately.
> 
> One of the things I've been ranting about lately is that Linux
> subsystem maintainers have become de-facto device tree
> standard commite chairs. :-(

This is the first I've heard this rant.  :(

To that end, I agree with you.  My frustration boiled down to trying to
predict the future, which is futile. :-P

For our scenario, once we can confirm our least popular kirkwood
variant, the 6282, behaves the same as we've seen so far, then
"marvell,kirkwood-eth" is fine by me.

> So to the actual question:
> 
> In general I think we need to draw a line and define what we
> mean with "describing the hardware" in a device tree.
> 
> We have some consensus:
> -  properties to describe regsiter BASE offset in physical
>   memory and size.
> - Resources like IRQ, DMA channel, regulator, GPIO pin control
>   handles, are passed using <&ersand> notation.
> 
> And so it goes on.
> 
> When it comes to defining different registers and their individual
> bits and the meaning of these and/or default values, I personally
> think that is making things harder for developers rather than
> simplifying things. I know that pinctrl-single is anyway doing this
> and I was talked into accepting it under circumstances where
> developers are being passed opaque machine-generated
> data that would otherwise be translated into unreadable header
> files littering the kernel.
> 
> For a coder it is definately better if the *driver* know these
> details, but whether that is possible seems to depend on things
> like hardware development process.

Agree.

> IMO: if you want to go down that road, what you really want is not
> ever more expressible device trees, but real open firmware,
> or ACPI or UEFI that can interpret and run bytecode as some
> "bios" for you. With DT coming from OF maybe this is a natural
> progression of things, but one has to realize when we reach the
> point where what we really want is a bios. Then your time is
> likely better spent with Tianocore or something than with the
> kernel.

shudder.  I like embedded systems because the *don't* have a bios.

thx,

Jason.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Jason Cooper
On Fri, May 24, 2013 at 12:40:26AM +0200, Sebastian Hesselbarth wrote:
> On 05/23/2013 08:40 PM, Jason Cooper wrote:
> >On Thu, May 23, 2013 at 11:53:57AM -0600, Jason Gunthorpe wrote:
> >>On Thu, May 23, 2013 at 01:23:39PM -0400, Jason Cooper wrote:
> >>>Shouldn't it rather be
> >>>
> >>>   compatible = "marvell,kirkwood-eth", "marvell,orion-eth";
> >>
> >>Not sure about orion-eth?

Sorry, yep, one or the other.

> Jason, Jason,

For a second, I read this as "tsk tsk tsk..." ;-)

> sorry I didn't came back to this conversation earlier. I already
> reworked the patch to rely on
> of_device_is_compatible(.."marvell,kirkwood-eth"..). This is a
> kirkwood only thing as other Orions cannot do clock gating or
> retain critcal register content (Dove). I will stick with orion-eth
> for all other and maybe introduce new compatible strings (and new
> fixes) as soon as issues surface.

Okay, as I mentioned to Jason, I would like to test 6282 before we
settle on this path.  Other than that, I'm fine with it.

> >>>I'm inclined to go with of_machine_is_compatible() since the only
> >>>concrete difference we know is that the tweak is needed on kirkwood and
> >>>nowhere else.
> >>
> >>But there is a larger problem here then just this one bit.
> >>
> >>The PSC1 register must be set properly for the board layout, and today
> >>we rely on the bootloader to set it. In fact, even with Sebastian's
> >>change the ethernet port won't work without bootloader
> >>intervention. The PortReset bit should also be cleared by the driver
> >>(and it is only present on some variants of this IP block,
> >>apparently).
> 
> Actually, fixing modular scenarios is only for the sake of multiarch
> someday. I don't see the point in running current kernel without eth
> compiled in _on a NAS SoC_ ;)

Good point, but if the eth can be gated to save power, we shouldn't
limit the user's ability just because the SoC is primarily in NAS's.

thx,

Jason.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Jason Cooper
On Thu, May 23, 2013 at 01:01:40PM -0600, Jason Gunthorpe wrote:
> On Thu, May 23, 2013 at 02:40:28PM -0400, Jason Cooper wrote:
> 
> > > But there is a larger problem here then just this one bit.
> > > 
> > > The PSC1 register must be set properly for the board layout, and today
> > > we rely on the bootloader to set it. In fact, even with Sebastian's
> > > change the ethernet port won't work without bootloader
> > > intervention. The PortReset bit should also be cleared by the driver
> > > (and it is only present on some variants of this IP block,
> > > apparently).
> > > 
> > > We know that some Marvell SOC's wack the ethernet registers when they
> > > clock gate, and the flip of Clk125Bypass is another symptom of this
> > > general problem.
> > > 
> > > So, long term, the PSC1 must be fully set by the driver, based on DT
> > > information describing the board (eg RGMII/MII/1000Base-X [SFP] Phy
> > > type), and the layout of this register seems to vary on a SOC by SOC
> > > basis.
> > > 
> > > Thus, I think it is appropriate to call this variant of the eth IP
> > > 'marvell,kirkwood-eth' which indicates that the register block follows
> > > the kirkwood manual and the PSC1 register specifically has the
> > > kirkwood layout.
> > 
> > Ok, so mv643xx_eth would match both "marvell,orion-eth" and
> > "marvell,kirkwood-eth", then write to PSC1 iff it sees a node matching
> > "marvell,kirkwood-eth".  I'm not too keen on that, however, the matching
> > of the machine doesn't look to good, either.
> 
> Why are you not keen on this? It seems like normal device driver
> practice, that is what the data field of of_device_id is typically
> used for..

I'm not keen on it because we don't have a document saying "All kirkwood
SoCs need PSC1 set to X after reset."  We know it, but have we tested
the 6282?

That being said, if "marvell,kirkwood-eth" is the best we can do for
now, I'm all for it.  I would just like to be reasonably certain that
the binding we are creating doesn't lock us into a difficult decision
later.

> There are more compatible strings than just kirkwood and orion in this
> driver, the whole TX_BW_CONTROL_OLD_LAYOUT/TX_BW_CONTROL_NEW_LAYOUT
> buisness (affecting PPC/MIPS) should also someday be captured with
> compatible strings rather than auto-detection too..

Agreed.

> > > The question is what other Marvell SOCs have the same PSC1 layout as
> > > kirkwood?
> > 
> > I think marvell,psc1_reset = <>; gives us the most flexibility in
> > accurately describing the hardware.
> 
> Agree, providing psc1_reset value is a good idea to setup the phy
> modes. If all 'orion' SOCs have the PSC1 value then we don't need the
> kirkwood differentiators, especially if things like the reset bit are
> in the same place.
> 
> The same trick Sebastian used to capture the mac address could be used
> to capture the PSC1 value from the bootloader.
> 
> Basically, I think any IP variants that have idential register layouts
> can share a compatible string, otherwise different layouts need
> different compatible strings, so the general format:
> 
>  compatible = "marvell,SOCNAME-eth", "marvell,-eth";
> 
> Seems very sane to me. At least this way if we discover more changes
> then the driver can match on the SOCNAME compatible string to find
> them.

After glancing a LinusW's email, I'm thinking this isn't the correct
path.  I'll write more in my response to him.

thx,

Jason.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v3 07/11] powerpc: uaccess s/might_sleep/might_fault/

2013-05-24 Thread Michael S. Tsirkin
The only reason uaccess routines might sleep
is if they fault. Make this explicit.

Arnd Bergmann suggested that the following code
if (!is_kernel_addr((unsigned long)__pu_addr))
might_fault();
can be further simplified by adding a version of might_fault
that includes the kernel addr check.

Will be considered as a further optimization in future.

Signed-off-by: Michael S. Tsirkin 
---
 arch/powerpc/include/asm/uaccess.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h 
b/arch/powerpc/include/asm/uaccess.h
index 4db4959..9485b43 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -178,7 +178,7 @@ do {
\
long __pu_err;  \
__typeof__(*(ptr)) __user *__pu_addr = (ptr);   \
if (!is_kernel_addr((unsigned long)__pu_addr))  \
-   might_sleep();  \
+   might_fault();  \
__chk_user_ptr(ptr);\
__put_user_size((x), __pu_addr, (size), __pu_err);  \
__pu_err;   \
@@ -188,7 +188,7 @@ do {
\
 ({ \
long __pu_err = -EFAULT;\
__typeof__(*(ptr)) __user *__pu_addr = (ptr);   \
-   might_sleep();  \
+   might_fault();  \
if (access_ok(VERIFY_WRITE, __pu_addr, size))   \
__put_user_size((x), __pu_addr, (size), __pu_err);  \
__pu_err;   \
@@ -268,7 +268,7 @@ do {
\
const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__chk_user_ptr(ptr);\
if (!is_kernel_addr((unsigned long)__gu_addr))  \
-   might_sleep();  \
+   might_fault();  \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
__gu_err;   \
@@ -282,7 +282,7 @@ do {
\
const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__chk_user_ptr(ptr);\
if (!is_kernel_addr((unsigned long)__gu_addr))  \
-   might_sleep();  \
+   might_fault();  \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
__gu_err;   \
@@ -294,7 +294,7 @@ do {
\
long __gu_err = -EFAULT;\
unsigned long  __gu_val = 0;\
const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
-   might_sleep();  \
+   might_fault();  \
if (access_ok(VERIFY_READ, __gu_addr, (size)))  \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
@@ -419,14 +419,14 @@ static inline unsigned long __copy_to_user_inatomic(void 
__user *to,
 static inline unsigned long __copy_from_user(void *to,
const void __user *from, unsigned long size)
 {
-   might_sleep();
+   might_fault();
return __copy_from_user_inatomic(to, from, size);
 }
 
 static inline unsigned long __copy_to_user(void __user *to,
const void *from, unsigned long size)
 {
-   might_sleep();
+   might_fault();
return __copy_to_user_inatomic(to, from, size);
 }
 
@@ -434,7 +434,7 @@ extern unsigned long __clear_user(void __user *addr, 
unsigned long size);
 
 static inline unsigned long clear_user(void __user *addr, unsigned long size)
 {
-   might_sleep();
+   might_fault();
if (likely(access_ok(VERIFY_WRITE, addr, size)))
return __clear_user(addr, size);
if ((unsigned long)addr < TASK_SIZE) {
-- 
MST
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.

Re: [PATCH 3/3] powerpc/vfio: Enable on pSeries platform

2013-05-24 Thread Alex Williamson
On Tue, 2013-05-21 at 13:33 +1000, Alexey Kardashevskiy wrote:
> The enables VFIO on the pSeries platform, enabling user space
> programs to access PCI devices directly.
> 
> Signed-off-by: Alexey Kardashevskiy 
> Cc: David Gibson 
> Signed-off-by: Paul Mackerras 

Acked-by: Alex Williamson 

> ---
>  arch/powerpc/platforms/pseries/iommu.c |4 
>  drivers/iommu/Kconfig  |2 +-
>  drivers/vfio/Kconfig   |2 +-
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c 
> b/arch/powerpc/platforms/pseries/iommu.c
> index 86ae364..23fc1dc 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -614,6 +614,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
>  
>   iommu_table_setparms(pci->phb, dn, tbl);
>   pci->iommu_table = iommu_init_table(tbl, pci->phb->node);
> + iommu_register_group(tbl, pci_domain_nr(bus), 0);
>  
>   /* Divide the rest (1.75GB) among the children */
>   pci->phb->dma_window_size = 0x8000ul;
> @@ -658,6 +659,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus 
> *bus)
>  ppci->phb->node);
>   iommu_table_setparms_lpar(ppci->phb, pdn, tbl, dma_window);
>   ppci->iommu_table = iommu_init_table(tbl, ppci->phb->node);
> + iommu_register_group(tbl, pci_domain_nr(bus), 0);
>   pr_debug("  created table: %p\n", ppci->iommu_table);
>   }
>  }
> @@ -684,6 +686,7 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
>  phb->node);
>   iommu_table_setparms(phb, dn, tbl);
>   PCI_DN(dn)->iommu_table = iommu_init_table(tbl, phb->node);
> + iommu_register_group(tbl, pci_domain_nr(phb->bus), 0);
>   set_iommu_table_base(&dev->dev, PCI_DN(dn)->iommu_table);
>   return;
>   }
> @@ -1184,6 +1187,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev 
> *dev)
>  pci->phb->node);
>   iommu_table_setparms_lpar(pci->phb, pdn, tbl, dma_window);
>   pci->iommu_table = iommu_init_table(tbl, pci->phb->node);
> + iommu_register_group(tbl, pci_domain_nr(pci->phb->bus), 0);
>   pr_debug("  created table: %p\n", pci->iommu_table);
>   } else {
>   pr_debug("  found DMA window, table: %p\n", pci->iommu_table);
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 3f3abde..01730b2 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -263,7 +263,7 @@ config SHMOBILE_IOMMU_L1SIZE
>  
>  config SPAPR_TCE_IOMMU
>   bool "sPAPR TCE IOMMU Support"
> - depends on PPC_POWERNV
> + depends on PPC_POWERNV || PPC_PSERIES
>   select IOMMU_API
>   help
> Enables bits of IOMMU API required by VFIO. The iommu_ops
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index b464687..26b3d9d 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -12,7 +12,7 @@ menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   depends on IOMMU_API
>   select VFIO_IOMMU_TYPE1 if X86
> - select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
> + select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
>   help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/vfio.txt for more details.



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/3] powerpc/vfio: Implement IOMMU driver for VFIO

2013-05-24 Thread Alex Williamson
On Tue, 2013-05-21 at 13:33 +1000, Alexey Kardashevskiy wrote:
> VFIO implements platform independent stuff such as
> a PCI driver, BAR access (via read/write on a file descriptor
> or direct mapping when possible) and IRQ signaling.
> 
> The platform dependent part includes IOMMU initialization
> and handling.  This implements an IOMMU driver for VFIO
> which does mapping/unmapping pages for the guest IO and
> provides information about DMA window (required by a POWER
> guest).
> 
> Cc: David Gibson 
> Signed-off-by: Alexey Kardashevskiy 
> Signed-off-by: Paul Mackerras 

Acked-by: Alex Williamson 

> ---
>  Documentation/vfio.txt  |   63 ++
>  drivers/vfio/Kconfig|6 +
>  drivers/vfio/Makefile   |1 +
>  drivers/vfio/vfio.c |1 +
>  drivers/vfio/vfio_iommu_spapr_tce.c |  377 
> +++
>  include/uapi/linux/vfio.h   |   34 
>  6 files changed, 482 insertions(+)
>  create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
> 
> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> index 8eda363..c55533c 100644
> --- a/Documentation/vfio.txt
> +++ b/Documentation/vfio.txt
> @@ -283,6 +283,69 @@ a direct pass through for VFIO_DEVICE_* ioctls.  The 
> read/write/mmap
>  interfaces implement the device region access defined by the device's
>  own VFIO_DEVICE_GET_REGION_INFO ioctl.
>  
> +
> +PPC64 sPAPR implementation note
> +---
> +
> +This implementation has some specifics:
> +
> +1) Only one IOMMU group per container is supported as an IOMMU group
> +represents the minimal entity which isolation can be guaranteed for and
> +groups are allocated statically, one per a Partitionable Endpoint (PE)
> +(PE is often a PCI domain but not always).
> +
> +2) The hardware supports so called DMA windows - the PCI address range
> +within which DMA transfer is allowed, any attempt to access address space
> +out of the window leads to the whole PE isolation.
> +
> +3) PPC64 guests are paravirtualized but not fully emulated. There is an API
> +to map/unmap pages for DMA, and it normally maps 1..32 pages per call and
> +currently there is no way to reduce the number of calls. In order to make 
> things
> +faster, the map/unmap handling has been implemented in real mode which 
> provides
> +an excellent performance which has limitations such as inability to do
> +locked pages accounting in real time.
> +
> +So 3 additional ioctls have been added:
> +
> + VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
> + of the DMA window on the PCI bus.
> +
> + VFIO_IOMMU_ENABLE - enables the container. The locked pages accounting
> + is done at this point. This lets user first to know what
> + the DMA window is and adjust rlimit before doing any real job.
> +
> + VFIO_IOMMU_DISABLE - disables the container.
> +
> +
> +The code flow from the example above should be slightly changed:
> +
> + .
> + /* Add the group to the container */
> + ioctl(group, VFIO_GROUP_SET_CONTAINER, &container);
> +
> + /* Enable the IOMMU model we want */
> + ioctl(container, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU)
> +
> + /* Get addition sPAPR IOMMU info */
> + vfio_iommu_spapr_tce_info spapr_iommu_info;
> + ioctl(container, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &spapr_iommu_info);
> +
> + if (ioctl(container, VFIO_IOMMU_ENABLE))
> + /* Cannot enable container, may be low rlimit */
> +
> + /* Allocate some space and setup a DMA mapping */
> + dma_map.vaddr = mmap(0, 1024 * 1024, PROT_READ | PROT_WRITE,
> +  MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> +
> + dma_map.size = 1024 * 1024;
> + dma_map.iova = 0; /* 1MB starting at 0x0 from device view */
> + dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
> +
> + /* Check here is .iova/.size are within DMA window from 
> spapr_iommu_info */
> +
> + ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
> + .
> +
>  
> ---
>  
>  [1] VFIO was originally an acronym for "Virtual Function I/O" in its
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 7cd5dec..b464687 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
>   depends on VFIO
>   default n
>  
> +config VFIO_IOMMU_SPAPR_TCE
> + tristate
> + depends on VFIO && SPAPR_TCE_IOMMU
> + default n
> +
>  menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   depends on IOMMU_API
>   select VFIO_IOMMU_TYPE1 if X86
> + select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
>   help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/vfio.txt for more details.
> diff --

Re: [PATCH v2 07/10] powerpc: uaccess s/might_sleep/might_fault/

2013-05-24 Thread Arnd Bergmann
On Friday 24 May 2013, Michael S. Tsirkin wrote:
> So this won't work, unless we add the is_kernel_addr check
> to might_fault. That will become possible on top of this patchset
> but let's consider this carefully, and make this a separate
> patchset, OK?

Yes, makes sense.

Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 07/10] powerpc: uaccess s/might_sleep/might_fault/

2013-05-24 Thread Michael S. Tsirkin
On Fri, May 24, 2013 at 04:00:32PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 22, 2013 at 03:59:01PM +0200, Arnd Bergmann wrote:
> > On Thursday 16 May 2013, Michael S. Tsirkin wrote:
> > > @@ -178,7 +178,7 @@ do {  
> > >   \
> > > long __pu_err;  \
> > > __typeof__(*(ptr)) __user *__pu_addr = (ptr);   \
> > > if (!is_kernel_addr((unsigned long)__pu_addr))  \
> > > -   might_sleep();  \
> > > +   might_fault();  \
> > > __chk_user_ptr(ptr);\
> > > __put_user_size((x), __pu_addr, (size), __pu_err);  \
> > > __pu_err;   \
> > > 
> > 
> > Another observation:
> > 
> > if (!is_kernel_addr((unsigned long)__pu_addr))
> > might_sleep();
> > 
> > is almost the same as
> > 
> > might_fault();
> > 
> > except that it does not call might_lock_read().
> > 
> > The version above may have been put there intentionally and correctly, but
> > if you want to replace it with might_fault(), you should remove the
> > "if ()" condition.
> > 
> > Arnd
> 
> Well not exactly. The non-inline might_fault checks the
> current segment, not the address.
> I'm guessing this is trying to do the same just without
> pulling in segment_eq, but I'd like a confirmation
> from more PPC maintainers.
> 
> Guys would you ack
> 
> - if (!is_kernel_addr((unsigned long)__pu_addr))
> - might_fault();
> + might_fault();
> 
> on top of this patch?

OK I spoke too fast: I found this:

powerpc: Fix incorrect might_sleep in __get_user/__put_user on kernel 
addresses

We have a case where __get_user and __put_user can validly be used
on kernel addresses in interrupt context - namely, the alignment
exception handler, as our get/put_unaligned just do a single access
and rely on the alignment exception handler to fix things up in the
rare cases where the cpu can't handle it in hardware.  Thus we can
get alignment exceptions in the network stack at interrupt level.
The alignment exception handler does a __get_user to read the
instruction and blows up in might_sleep().

Since a __get_user on a kernel address won't actually ever sleep,
this makes the might_sleep conditional on the address being less
than PAGE_OFFSET.

Signed-off-by: Paul Mackerras 

So this won't work, unless we add the is_kernel_addr check
to might_fault. That will become possible on top of this patchset
but let's consider this carefully, and make this a separate
patchset, OK?

> Also, any volunteer to test this (not just test-build)?
> 
> -- 
> MST
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 07/10] powerpc: uaccess s/might_sleep/might_fault/

2013-05-24 Thread Michael S. Tsirkin
On Wed, May 22, 2013 at 03:59:01PM +0200, Arnd Bergmann wrote:
> On Thursday 16 May 2013, Michael S. Tsirkin wrote:
> > @@ -178,7 +178,7 @@ do {
> > \
> > long __pu_err;  \
> > __typeof__(*(ptr)) __user *__pu_addr = (ptr);   \
> > if (!is_kernel_addr((unsigned long)__pu_addr))  \
> > -   might_sleep();  \
> > +   might_fault();  \
> > __chk_user_ptr(ptr);\
> > __put_user_size((x), __pu_addr, (size), __pu_err);  \
> > __pu_err;   \
> > 
> 
> Another observation:
> 
>   if (!is_kernel_addr((unsigned long)__pu_addr))
>   might_sleep();
> 
> is almost the same as
> 
>   might_fault();
> 
> except that it does not call might_lock_read().
> 
> The version above may have been put there intentionally and correctly, but
> if you want to replace it with might_fault(), you should remove the
> "if ()" condition.
> 
>   Arnd

Well not exactly. The non-inline might_fault checks the
current segment, not the address.
I'm guessing this is trying to do the same just without
pulling in segment_eq, but I'd like a confirmation
from more PPC maintainers.

Guys would you ack

-   if (!is_kernel_addr((unsigned long)__pu_addr))
-   might_fault();
+   might_fault();

on top of this patch?

Also, any volunteer to test this (not just test-build)?

-- 
MST
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

2013-05-24 Thread Linus Walleij
On Fri, May 24, 2013 at 12:40 AM, Sebastian Hesselbarth
 wrote:
> On 05/23/2013 08:40 PM, Jason Cooper wrote:

>> I think marvell,psc1_reset =<>; gives us the most flexibility in
>> accurately describing the hardware.
>
>
> IMHO using that is just another workaround for a broken driver. We
> could hack the whole register setup in DT as it would still accurately
> describe HW. Don't get me wrong, but I don't like it.
>
> Haven't checked how happy Linus Walleij is about pinctrl drivers with
> reg values hacked in lately.

One of the things I've been ranting about lately is that Linux
subsystem maintainers have become de-facto device tree
standard commite chairs. :-(

So to the actual question:

In general I think we need to draw a line and define what we
mean with "describing the hardware" in a device tree.

We have some consensus:
-  properties to describe regsiter BASE offset in physical
  memory and size.
- Resources like IRQ, DMA channel, regulator, GPIO pin control
  handles, are passed using <&ersand> notation.

And so it goes on.

When it comes to defining different registers and their individual
bits and the meaning of these and/or default values, I personally
think that is making things harder for developers rather than
simplifying things. I know that pinctrl-single is anyway doing this
and I was talked into accepting it under circumstances where
developers are being passed opaque machine-generated
data that would otherwise be translated into unreadable header
files littering the kernel.

For a coder it is definately better if the *driver* know these
details, but whether that is possible seems to depend on things
like hardware development process.

IMO: if you want to go down that road, what you really want is not
ever more expressible device trees, but real open firmware,
or ACPI or UEFI that can interpret and run bytecode as some
"bios" for you. With DT coming from OF maybe this is a natural
progression of things, but one has to realize when we reach the
point where what we really want is a bios. Then your time is
likely better spent with Tianocore or something than with the
kernel.

Yours,
Linus Walleij
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[git pull] Please pull powerpc.git merge branch

2013-05-24 Thread Benjamin Herrenschmidt
Hi Linus !

Here are a few more powerpc fixes for 3.10. Some more P8 related
bits, a bunch of fixes for our P7+/P8 HW crypto drivers, some added
workarounds for those radeons that don't do proper 64-bit MSIs and
a couple of other trivialities by myself.

Cheers,
Ben.

The following changes since commit 519fe2ecb755b875d9814cdda19778c2e88c6901:

  Merge branch 'leds-fixes-3.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/cooloney/linux-leds (2013-05-21 
11:41:07 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

for you to fetch changes up to f1dd153121dcb872ae6cba8d52bec97519eb7d97:

  powerpc/pseries: Make 32-bit MSI quirk work on systems lacking firmware 
support (2013-05-24 18:16:54 +1000)


Benjamin Herrenschmidt (5):
  powerpc: Fix TLB cleanup at boot on POWER8
  powerpc/pci: Fix bogus message at boot about empty memory resources
  powerpc/powernv: Fix condition for when to invalidate the TCE cache
  powerpc: Make radeon 32-bit MSI quirk work on powernv
  powerpc/powernv: Build a zImage.epapr

Brian King (1):
  powerpc/pseries: Make 32-bit MSI quirk work on systems lacking firmware 
support

Kent Yoder (1):
  drivers/crypto/nx: Fixes for multiple races and issues

Michael Ellerman (1):
  powerpc: Context switch more PMU related SPRs

 arch/powerpc/include/asm/pci-bridge.h |2 +
 arch/powerpc/include/asm/processor.h  |6 +++
 arch/powerpc/kernel/asm-offsets.c |6 +++
 arch/powerpc/kernel/cpu_setup_power.S |8 ++-
 arch/powerpc/kernel/entry_64.S|   28 +++
 arch/powerpc/kernel/pci-common.c  |7 +--
 arch/powerpc/kernel/pci_64.c  |   10 
 arch/powerpc/kernel/pci_dn.c  |8 +++
 arch/powerpc/platforms/powernv/Kconfig|1 +
 arch/powerpc/platforms/powernv/pci-ioda.c |   27 +--
 arch/powerpc/platforms/powernv/pci.c  |6 ++-
 arch/powerpc/platforms/pseries/msi.c  |   75 +++--
 drivers/crypto/nx/nx-aes-cbc.c|1 +
 drivers/crypto/nx/nx-aes-ecb.c|1 +
 drivers/crypto/nx/nx-aes-gcm.c|2 +-
 drivers/crypto/nx/nx-sha256.c |8 +--
 drivers/crypto/nx/nx-sha512.c |7 +--
 drivers/crypto/nx/nx.c|   38 +++
 18 files changed, 146 insertions(+), 95 deletions(-)


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev