LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: SB600 for the Nemo board has non-zero devices on non-root bus
From: Michael Ellerman @ 2018-06-04 14:10 UTC (permalink / raw)
  To: Christian Zigotzky, Olof Johansson, Bjorn Helgaas,
	linux-pci@vger.kernel.org, Darren Stevens, Bjorn Helgaas,
	linuxppc-dev
In-Reply-To: <99f78122-cd53-100f-4f58-be5bf7927d4e@xenosoft.de>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]

On Wed, 2017-12-06 at 11:03:52 UTC, Christian Zigotzky wrote:
> On 06 December 2017 at 09:37AM, Christian Zigotzky wrote:
>  > On 03 December 2017 at 10:43AM, Christian Zigotzky wrote:
>  > >
>  > > On 3. Dec 2017, at 00:02, Olof Johansson <olof@lixom.net> wrote:
>  > >>
>  > >> Typo, should be ';', not ':'. I obviously didn't even try 
> compiling this. :)
>  > >>
>  > >>
>  > >> -Olof
>  > >
>  > > Hi Olof,
>  > >
>  > > Thanks a lot for your patch! I will test it on Wednesday.
>  > >
>  > > Cheers,
>  > > Christian
>  >
>  >
>  > Hi Olof,
>  >
>  > I tested your patch today. Unfortunately the kernel 4.15-rc2 doesn't 
> compile with your patch.
>  >
>  > Error messages:
>  >
>  >                        ^~~~~~~~~
>  > arch/powerpc/platforms/pasemi/pci.c: In function ‘pas_pci_init’:
>  > arch/powerpc/platforms/pasemi/pci.c:298:2: error: implicit 
> declaration of function ‘pci_set_flag’ 
> [-Werror=implicit-function-declaration]
>  >   pci_set_flag(PCI_SCAN_ALL_PCIE_DEVS);
>  >   ^~~~~~~~~~~~
>  > cc1: some warnings being treated as errors
>  >
>  > ---
>  >
>  > I figured out that we need 'pci_set_flags' instead of 'pci_set_flag'. 
> I modified your patch and after that the kernel compiles. Please find 
> attached the new patch.
>  >
>  > Cheers,
>  > Christian
> 
> Hi Olof,
> 
> Many thanks for your patch! :-) The RC2 of kernel 4.15 boots without any 
> problems on my P.A. Semi Nemo board (A-EON AmigaOne X1000). I don’t need 
> the additional boot argument 'pci=pcie_scan_all' anymore.
> 
> Is it possible to merge it via the powerpc tree?
> 
> Thanks,
> Christian
> 
> arch/powerpc/platforms/pasemi/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
> index 5ff6108..ea54ed2 100644
> --- a/arch/powerpc/platforms/pasemi/pci.c
> +++ b/arch/powerpc/platforms/pasemi/pci.c
> @@ -224,6 +224,8 @@ void __init pas_pci_init(void)
>  		return;
>  	}
>  
> +	pci_set_flags(PCI_SCAN_ALL_PCIE_DEVS);
> +
>  	for (np = NULL; (np = of_get_next_child(root, np)) != NULL;)
>  		if (np->name && !strcmp(np->name, "pxp") && !pas_add_bridge(np))
>  			of_node_get(np);

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/eff06ef0891d200eb0ddd156c6e96c

cheers

^ permalink raw reply

* Re: pkeys on POWER: Access rights not reset on execve
From: Ram Pai @ 2018-06-04 14:01 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Andy Lutomirski, Linux-MM, linuxppc-dev, Dave Hansen
In-Reply-To: <4e53b91f-80a7-816a-3e9b-56d7be7cd092@redhat.com>

On Mon, Jun 04, 2018 at 12:12:07PM +0200, Florian Weimer wrote:
> On 06/03/2018 10:18 PM, Ram Pai wrote:
> >On Mon, May 21, 2018 at 01:29:11PM +0200, Florian Weimer wrote:
> >>On 05/20/2018 09:11 PM, Ram Pai wrote:
> >>>Florian,
> >>>
> >>>	Does the following patch fix the problem for you?  Just like x86
> >>>	I am enabling all keys in the UAMOR register during
> >>>	initialization itself. Hence any key created by any thread at
> >>>	any time, will get activated on all threads. So any thread
> >>>	can change the permission on that key. Smoke tested it
> >>>	with your test program.
> >>
> >>I think this goes in the right direction, but the AMR value after
> >>fork is still strange:
> >>
> >>AMR (PID 34912): 0x0000000000000000
> >>AMR after fork (PID 34913): 0x0000000000000000
> >>AMR (PID 34913): 0x0000000000000000
> >>Allocated key in subprocess (PID 34913): 2
> >>Allocated key (PID 34912): 2
> >>Setting AMR: 0xffffffffffffffff
> >>New AMR value (PID 34912): 0x0fffffffffffffff
> >>About to call execl (PID 34912) ...
> >>AMR (PID 34912): 0x0fffffffffffffff
> >>AMR after fork (PID 34914): 0x0000000000000003
> >>AMR (PID 34914): 0x0000000000000003
> >>Allocated key in subprocess (PID 34914): 2
> >>Allocated key (PID 34912): 2
> >>Setting AMR: 0xffffffffffffffff
> >>New AMR value (PID 34912): 0x0fffffffffffffff
> >>
> >>I mean this line:
> >>
> >>AMR after fork (PID 34914): 0x0000000000000003
> >>
> >>Shouldn't it be the same as in the parent process?
> >
> >Fixed it. Please try this patch. If it all works to your satisfaction, I
> >will clean it up further and send to Michael Ellermen(ppc maintainer).
> >
> >
> >commit 51f4208ed5baeab1edb9b0f8b68d7144449b3527
> >Author: Ram Pai <linuxram@us.ibm.com>
> >Date:   Sun Jun 3 14:44:32 2018 -0500
> >
> >     Fix for the fork bug.
> >     Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> 
> Is this on top of the previous patch, or a separate fix?

top of previous patch.
RP

^ permalink raw reply

* Re: [PATCH 09/10] dpaa_eth: add support for hardware timestamping
From: Richard Cochran @ 2018-06-04 13:49 UTC (permalink / raw)
  To: Yangbo Lu
  Cc: netdev, madalin.bucur, Rob Herring, Shawn Guo, David S . Miller,
	devicetree, linuxppc-dev, linux-arm-kernel, linux-kernel
In-Reply-To: <20180604070837.19265-10-yangbo.lu@nxp.com>

On Mon, Jun 04, 2018 at 03:08:36PM +0800, Yangbo Lu wrote:

> +if FSL_DPAA_ETH
> +config FSL_DPAA_ETH_TS
> +	bool "DPAA hardware timestamping support"
> +	select PTP_1588_CLOCK_QORIQ
> +	default n
> +	help
> +	  Enable DPAA hardware timestamping support.
> +	  This option is useful for applications to get
> +	  hardware time stamps on the Ethernet packets
> +	  using the SO_TIMESTAMPING API.
> +endif

You should drop this #ifdef.  In general, if a MAC supports time
stamping and PHC, then the driver support should simply be compiled
in.

[ When time stamping incurs a large run time performance penalty to
  non-PTP users, then it might make sense to have a Kconfig option to
  disable it, but that doesn't appear to be the case here. ]

> @@ -1615,6 +1635,24 @@ static int dpaa_eth_refill_bpools(struct dpaa_priv *priv)
>  	skbh = (struct sk_buff **)phys_to_virt(addr);
>  	skb = *skbh;
>  
> +#ifdef CONFIG_FSL_DPAA_ETH_TS
> +	if (priv->tx_tstamp &&
> +	    skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {

This condition fits on one line easily.

> +		struct skb_shared_hwtstamps shhwtstamps;
> +		u64 ns;

Local variables belong at the top of the function.

> +		memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> +
> +		if (!dpaa_get_tstamp_ns(priv->net_dev, &ns,
> +					priv->mac_dev->port[TX],
> +					(void *)skbh)) {
> +			shhwtstamps.hwtstamp = ns_to_ktime(ns);
> +			skb_tstamp_tx(skb, &shhwtstamps);
> +		} else {
> +			dev_warn(dev, "dpaa_get_tstamp_ns failed!\n");
> +		}
> +	}
> +#endif
>  	if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
>  		nr_frags = skb_shinfo(skb)->nr_frags;
>  		dma_unmap_single(dev, addr, qm_fd_get_offset(fd) +
> @@ -2086,6 +2124,14 @@ static int dpaa_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
>  	if (unlikely(err < 0))
>  		goto skb_to_fd_failed;
>  
> +#ifdef CONFIG_FSL_DPAA_ETH_TS
> +	if (priv->tx_tstamp &&
> +	    skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {

One line please.

> +		fd.cmd |= FM_FD_CMD_UPD;
> +		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +	}
> +#endif
> +
>  	if (likely(dpaa_xmit(priv, percpu_stats, queue_mapping, &fd) == 0))
>  		return NETDEV_TX_OK;
>  

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
From: Julia Lawall @ 2018-06-04 13:46 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Qiang Zhao, Li Yang, linuxppc-dev, linux-kernel, kernel-janitors
In-Reply-To: <20180604134024.dmd7mn2wggqpmkcq@mwanda>



On Mon, 4 Jun 2018, Dan Carpenter wrote:

> On Mon, Jun 04, 2018 at 10:25:14PM +0900, Julia Lawall wrote:
> >
> >
> > On Mon, 4 Jun 2018, Dan Carpenter wrote:
> >
> > > There is a copy and paste bug so we accidentally use the RX_ shift when
> > > we're in TX_ mode.
> > >
> > > Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > Static analysis work.  Not tested.  This affects the success path, so
> > > we should probably test it.
> >
> > Maybe this is another one?  I don't have time to look into it at the
> > moment...
> >
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> >
> >  	/* For strict priority entries defines the number of consecutive
> >  	 * slots for the highest priority.
> >  	 */
> > 	REG_WR(bp, (port) ? NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS :
> > 		   NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS, 0x100);
> >  	/* Mapping between the CREDIT_WEIGHT registers and actual client
> >  	 * numbers
> >  	 */
> >
> > I find some others that choose between constants, such as ... ? 0 : 0.
>
> I feel like it should warn about all of those because people shouldn't
> be submitting unfinished written code to the kernel.  Coccinelle is a
> lot better for this than Smatch is because it's pre-processor stuff.

OK, maybe I can report these in the next few days.

thanks,
julia

^ permalink raw reply

* Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
From: Dan Carpenter @ 2018-06-04 13:40 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Qiang Zhao, Li Yang, linuxppc-dev, linux-kernel, kernel-janitors
In-Reply-To: <alpine.DEB.2.20.1806042223580.3542@hadrien>

On Mon, Jun 04, 2018 at 10:25:14PM +0900, Julia Lawall wrote:
> 
> 
> On Mon, 4 Jun 2018, Dan Carpenter wrote:
> 
> > There is a copy and paste bug so we accidentally use the RX_ shift when
> > we're in TX_ mode.
> >
> > Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Static analysis work.  Not tested.  This affects the success path, so
> > we should probably test it.
> 
> Maybe this is another one?  I don't have time to look into it at the
> moment...
> 
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> 
>  	/* For strict priority entries defines the number of consecutive
>  	 * slots for the highest priority.
>  	 */
> 	REG_WR(bp, (port) ? NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS :
> 		   NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS, 0x100);
>  	/* Mapping between the CREDIT_WEIGHT registers and actual client
>  	 * numbers
>  	 */
> 
> I find some others that choose between constants, such as ... ? 0 : 0.

I feel like it should warn about all of those because people shouldn't
be submitting unfinished written code to the kernel.  Coccinelle is a
lot better for this than Smatch is because it's pre-processor stuff.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
From: Julia Lawall @ 2018-06-04 13:25 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Qiang Zhao, Li Yang, linuxppc-dev, linux-kernel, kernel-janitors
In-Reply-To: <20180604115842.7c4vzge3igjbnblt@kili.mountain>



On Mon, 4 Jun 2018, Dan Carpenter wrote:

> There is a copy and paste bug so we accidentally use the RX_ shift when
> we're in TX_ mode.
>
> Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analysis work.  Not tested.  This affects the success path, so
> we should probably test it.

Maybe this is another one?  I don't have time to look into it at the
moment...

drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c

 	/* For strict priority entries defines the number of consecutive
 	 * slots for the highest priority.
 	 */
	REG_WR(bp, (port) ? NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS :
		   NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS, 0x100);
 	/* Mapping between the CREDIT_WEIGHT registers and actual client
 	 * numbers
 	 */

I find some others that choose between constants, such as ... ? 0 : 0.

julia


>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index c646d8713861..681f7d4b7724 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 tdm_num)
>  {
>  	u32 shift;
>
> -	shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
> +	shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : TX_SYNC_SHIFT_BASE;
>  	shift -= tdm_num * 2;
>
>  	return shift;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* [PATCH net-next] wan/fsl_ucc_hdlc: use dma_zalloc_coherent instead of allocator/memset
From: YueHaibing @ 2018-06-04 13:07 UTC (permalink / raw)
  To: davem, qiang.zhao; +Cc: netdev, linux-kernel, linuxppc-dev, YueHaibing

Use dma_zalloc_coherent instead of dma_alloc_coherent
followed by memset 0.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 33df764..4205dfd 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -270,10 +270,10 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 	iowrite16be(DEFAULT_HDLC_ADDR, &priv->ucc_pram->haddr4);
 
 	/* Get BD buffer */
-	bd_buffer = dma_alloc_coherent(priv->dev,
-				       (RX_BD_RING_LEN + TX_BD_RING_LEN) *
-				       MAX_RX_BUF_LENGTH,
-				       &bd_dma_addr, GFP_KERNEL);
+	bd_buffer = dma_zalloc_coherent(priv->dev,
+					(RX_BD_RING_LEN + TX_BD_RING_LEN) *
+					MAX_RX_BUF_LENGTH,
+					&bd_dma_addr, GFP_KERNEL);
 
 	if (!bd_buffer) {
 		dev_err(priv->dev, "Could not allocate buffer descriptors\n");
@@ -281,9 +281,6 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 		goto free_tiptr;
 	}
 
-	memset(bd_buffer, 0, (RX_BD_RING_LEN + TX_BD_RING_LEN)
-			* MAX_RX_BUF_LENGTH);
-
 	priv->rx_buffer = bd_buffer;
 	priv->tx_buffer = bd_buffer + RX_BD_RING_LEN * MAX_RX_BUF_LENGTH;
 
-- 
2.7.0

^ permalink raw reply related

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Christoph Hellwig @ 2018-06-04 12:55 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Benjamin Herrenschmidt, Anshuman Khandual, virtualization,
	linux-kernel, linuxppc-dev, aik, robh, joe, elfring, david,
	jasowang, mpe, hch
In-Reply-To: <20180604153558-mutt-send-email-mst@kernel.org>

On Mon, Jun 04, 2018 at 03:43:09PM +0300, Michael S. Tsirkin wrote:
> Another is that given the basic functionality is in there, optimizations
> can possibly wait until per-device quirks in DMA API are supported.

We have had per-device dma_ops for quite a while.

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Benjamin Herrenschmidt @ 2018-06-04 13:11 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Anshuman Khandual, virtualization, linux-kernel, linuxppc-dev,
	aik, robh, joe, elfring, david, jasowang, mpe, hch
In-Reply-To: <20180604153558-mutt-send-email-mst@kernel.org>

On Mon, 2018-06-04 at 15:43 +0300, Michael S. Tsirkin wrote:
> On Thu, May 24, 2018 at 08:27:04AM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2018-05-23 at 21:50 +0300, Michael S. Tsirkin wrote:
> > 
> > > I re-read that discussion and I'm still unclear on the
> > > original question, since I got several apparently
> > > conflicting answers.
> > > 
> > > I asked:
> > > 
> > > 	Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the
> > > 	hypervisor side sufficient?
> > 
> > I thought I had replied to this...
> > 
> > There are a couple of reasons:
> > 
> > - First qemu doesn't know that the guest will switch to "secure mode"
> > in advance. There is no difference between a normal and a secure
> > partition until the partition does the magic UV call to "enter secure
> > mode" and qemu doesn't see any of it. So who can set the flag here ?
> 
> The user should set it. You just tell user "to be able to use with
> feature X, enable IOMMU".

That's completely backwards. The user has no idea what that stuff is.
And it would have to percolate all the way up the management stack,
libvirt, kimchi, whatever else ... that's just nonsense.

Especially since, as I explained in my other email, this is *not* a
qemu problem and thus the solution shouldn't be messing around with
qemu.

> 
> > - Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
> > vhost) go through the emulated MMIO for every access to the guest,
> > which adds additional overhead.
> > 
> > Cheers,
> > Ben.
> 
> There are several answers to this.  One is that we are working hard to
> make overhead small when the mappings are static (which they would be if
> there's no actual IOMMU). So maybe especially given you are using
> a bounce buffer on top it's not so bad - did you try to
> benchmark?
> 
> Another is that given the basic functionality is in there, optimizations
> can possibly wait until per-device quirks in DMA API are supported.

The point is that requiring specific qemu command line arguments isn't
going to fly. We have additional problems due to the fact that our
firmware (SLOF) inside qemu doesn't currently deal with iommu's etc...
though those can be fixed.

Overall, however, this seems to be the most convoluted way of achieving
things, require user interventions where none should be needed etc...

Again, what's wrong with a 2 lines hook instead that solves it all and
completely avoids involving qemu ?

Ben.

> 
> > > 
> > > 
> > > >  arch/powerpc/include/asm/dma-mapping.h |  6 ++++++
> > > >  arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++
> > > >  drivers/virtio/virtio_ring.c           | 10 ++++++++++
> > > >  3 files changed, 27 insertions(+)
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> > > > index 8fa3945..056e578 100644
> > > > --- a/arch/powerpc/include/asm/dma-mapping.h
> > > > +++ b/arch/powerpc/include/asm/dma-mapping.h
> > > > @@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev);
> > > >  #define ARCH_HAS_DMA_MMAP_COHERENT
> > > >  
> > > >  #endif /* __KERNEL__ */
> > > > +
> > > > +#define platform_forces_virtio_dma platform_forces_virtio_dma
> > > > +
> > > > +struct virtio_device;
> > > > +
> > > > +extern bool platform_forces_virtio_dma(struct virtio_device *vdev);
> > > >  #endif	/* _ASM_DMA_MAPPING_H */
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > > > index 06f0296..a2ec15a 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -38,6 +38,7 @@
> > > >  #include <linux/of.h>
> > > >  #include <linux/iommu.h>
> > > >  #include <linux/rculist.h>
> > > > +#include <linux/virtio.h>
> > > >  #include <asm/io.h>
> > > >  #include <asm/prom.h>
> > > >  #include <asm/rtas.h>
> > > > @@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str)
> > > >  __setup("multitce=", disable_multitce);
> > > >  
> > > >  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> > > > +
> > > > +bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > > > +{
> > > > +	/*
> > > > +	 * On protected guest platforms, force virtio core to use DMA
> > > > +	 * MAP API for all virtio devices. But there can also be some
> > > > +	 * exceptions for individual devices like virtio balloon.
> > > > +	 */
> > > > +	return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
> > > > +}
> > > 
> > > Isn't this kind of slow?  vring_use_dma_api is on
> > > data path and supposed to be very fast.
> > > 
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 21d464a..47ea6c3 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -141,8 +141,18 @@ struct vring_virtqueue {
> > > >   * unconditionally on data path.
> > > >   */
> > > >  
> > > > +#ifndef platform_forces_virtio_dma
> > > > +static inline bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > > > +{
> > > > +	return false;
> > > > +}
> > > > +#endif
> > > > +
> > > >  static bool vring_use_dma_api(struct virtio_device *vdev)
> > > >  {
> > > > +	if (platform_forces_virtio_dma(vdev))
> > > > +		return true;
> > > > +
> > > >  	if (!virtio_has_iommu_quirk(vdev))
> > > >  		return true;
> > > >  
> > > > -- 
> > > > 2.9.3

^ permalink raw reply

* Re: [RFC PATCH v2 00/14] Remove unneccessary included headers
From: Michael Ellerman @ 2018-06-04 12:52 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <cover.1527868006.git.christophe.leroy@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> The purpose of this serie is to limit the number of includes to
> only the necessary ones in order to reduce the number of files
> recompiled everytime a header file is modified.
>
> This is the start of the work, please provide feedback if any so
> that I don't go in the wrong direction.
>
> Handled inclusion changes more carrefully after Michael feedback.
>
> Started splitting some headers in order to reduce their coverage.
>
> Christophe Leroy (14):
>   powerpc: remove kdump.h from page.h
>   powerpc: remove unneeded inclusions of cpu_has_feature.h
>   powerpc/405: move PPC405_ERR77 in asm-405.h
>   powerpc: move ASM_CONST and stringify_in_c() into asm-const.h
>   powerpc: clean the inclusion of stringify.h
>   powerpc: clean inclusions of asm/feature-fixups.h
>   powerpc: remove superflous inclusions of asm/fixmap.h
>   powerpc: declare set_breakpoint() static
>   powerpc/book3s: Remove PPC_PIN_SIZE
>   powerpc: fix includes in asm/processor.h
>   powerpc/nohash: fix hash related comments in pgtable.h
>   powerpc/44x: remove page.h from mmu-44x.h
>   powerpc: split reg.h in two parts
>   powerpc: Split synch.h in two parts

Still some problems :)

  http://kisskb.ellerman.id.au/kisskb/head/14047/

cheers


arch/powerpc/include/asm/reg.h:1286:31: error: expected ':' or ')' before 'ASM_FTR_IFCLR':
  allmodconfig+64K_PAGES powerpc
  allmodconfig+64K_PAGES powerpc-5.3
  allmodconfig+ppc64le ppc64le
  powernv_defconfig+NO_NUMA ppc64le
  powernv_defconfig+NO_PERF ppc64le
  powernv_defconfig+NO_RADIX ppc64le
  powernv_defconfig+STRICT_RWX ppc64le
  powernv_defconfig+THIN ppc64le
  powerpc-allmodconfig powerpc
  powerpc-allmodconfig powerpc-5.3
  powerpc-allyesconfig powerpc
  powerpc-allyesconfig powerpc-5.3
  ppc64_defconfig powerpc
  ppc64_defconfig powerpc-5.3
  ppc64_defconfig+NO_ALTIVEC powerpc
  ppc64_defconfig+NO_ALTIVEC powerpc-5.3
  ppc64_defconfig+NO_HUGETLB powerpc
  ppc64_defconfig+NO_HUGETLB powerpc-5.3
  ppc64_defconfig+NO_KVM powerpc
  ppc64_defconfig+NO_KVM powerpc-5.3
  ppc64_defconfig+NO_RADIX powerpc
  ppc64_defconfig+NO_TM powerpc
  ppc64_defconfig+NO_TM powerpc-5.3
  ppc64_defconfig+UP powerpc
  ppc64_defconfig+UP powerpc-5.3
  ppc64e_defconfig powerpc
  ppc64e_defconfig powerpc-5.3
  ppc64e_defconfig+KEXEC powerpc
  ppc64e_defconfig+KEXEC powerpc-5.3
  ppc64e_defconfig+UP powerpc
  ppc64e_defconfig+UP powerpc-5.3
  ppc64le_defconfig ppc64le
  ppc64le_defconfig+NO_KPROBES ppc64le
  ppc64le_defconfig+NO_KVM ppc64le
  ppc6xx_defconfig powerpc
  ppc6xx_defconfig powerpc-5.3
  pseries_defconfig powerpc
  pseries_defconfig powerpc-5.3
  pseries_defconfig+FA_DUMP powerpc
  pseries_defconfig+FA_DUMP powerpc-5.3
  pseries_defconfig+NO_MEMORY_HOTPLUG powerpc
  pseries_defconfig+NO_MEMORY_HOTPLUG powerpc-5.3
  pseries_defconfig+NO_MEMORY_HOTREMOVE powerpc
  pseries_defconfig+NO_SPLPAR powerpc
  pseries_defconfig+NO_SPLPAR powerpc-5.3
  pseries_le_defconfig ppc64le
  pseries_le_defconfig+NO_NUMA ppc64le
  pseries_le_defconfig+NO_SPLPAR ppc64le
  skiroot_defconfig ppc64le

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Michael S. Tsirkin @ 2018-06-04 12:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: David Gibson, Anshuman Khandual, virtualization, linux-kernel,
	linuxppc-dev, aik, robh, joe, elfring, jasowang, mpe, hch
In-Reply-To: <d5df613d6347fe2f9bb6ea65bc6f6be05650ca6f.camel@kernel.crashing.org>

On Mon, Jun 04, 2018 at 07:48:54PM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2018-06-04 at 18:57 +1000, David Gibson wrote:
> > 
> > > - First qemu doesn't know that the guest will switch to "secure mode"
> > > in advance. There is no difference between a normal and a secure
> > > partition until the partition does the magic UV call to "enter secure
> > > mode" and qemu doesn't see any of it. So who can set the flag here ?
> > 
> > This seems weird to me.  As a rule HV calls should go through qemu -
> > or be allowed to go directly to KVM *by* qemu.
> 
> It's not an HV call, it's a UV call, qemu won't see it, qemu isn't
> trusted. Now the UV *will* reflect that to the HV via some synthetized
> HV calls, and we *could* have those do a pass by qemu, however, so far,
> our entire design doesn't rely on *any* qemu knowledge whatsoever and
> it would be sad to add it just for that purpose.

It's a temporary work-around. I think that the long-term fix is to
support per-device quirks and have the DMA API DTRT for virtio.

> Additionally, this is rather orthogonal, see my other email, the
> problem we are trying to solve is *not* a qemu problem and it doesn't
> make sense to leak that into qemu.
> 
> >   We generally reserve
> > the latter for hot path things.  Since this isn't a hot path, having
> > the call handled directly by the kernel seems wrong.
> >
> > Unless a "UV call" is something different I don't know about.
> 
> Yes, a UV call goes to the Ultravisor, not the Hypervisor. The
> Hypervisor isn't trusted.
> 
> > > - Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
> > > vhost) go through the emulated MMIO for every access to the guest,
> > > which adds additional overhead.
> > 
> Ben.

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Michael S. Tsirkin @ 2018-06-04 12:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Anshuman Khandual, virtualization, linux-kernel, linuxppc-dev,
	aik, robh, joe, elfring, david, jasowang, mpe, hch
In-Reply-To: <f77638ddef3af52dd71341083707c9e3745dd505.camel@kernel.crashing.org>

On Thu, May 24, 2018 at 08:27:04AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2018-05-23 at 21:50 +0300, Michael S. Tsirkin wrote:
> 
> > I re-read that discussion and I'm still unclear on the
> > original question, since I got several apparently
> > conflicting answers.
> > 
> > I asked:
> > 
> > 	Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the
> > 	hypervisor side sufficient?
> 
> I thought I had replied to this...
> 
> There are a couple of reasons:
> 
> - First qemu doesn't know that the guest will switch to "secure mode"
> in advance. There is no difference between a normal and a secure
> partition until the partition does the magic UV call to "enter secure
> mode" and qemu doesn't see any of it. So who can set the flag here ?

The user should set it. You just tell user "to be able to use with
feature X, enable IOMMU".

> - Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
> vhost) go through the emulated MMIO for every access to the guest,
> which adds additional overhead.
> 
> Cheers,
> Ben.

There are several answers to this.  One is that we are working hard to
make overhead small when the mappings are static (which they would be if
there's no actual IOMMU). So maybe especially given you are using
a bounce buffer on top it's not so bad - did you try to
benchmark?

Another is that given the basic functionality is in there, optimizations
can possibly wait until per-device quirks in DMA API are supported.


> > 
> > 
> > >  arch/powerpc/include/asm/dma-mapping.h |  6 ++++++
> > >  arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++
> > >  drivers/virtio/virtio_ring.c           | 10 ++++++++++
> > >  3 files changed, 27 insertions(+)
> > > 
> > > diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> > > index 8fa3945..056e578 100644
> > > --- a/arch/powerpc/include/asm/dma-mapping.h
> > > +++ b/arch/powerpc/include/asm/dma-mapping.h
> > > @@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev);
> > >  #define ARCH_HAS_DMA_MMAP_COHERENT
> > >  
> > >  #endif /* __KERNEL__ */
> > > +
> > > +#define platform_forces_virtio_dma platform_forces_virtio_dma
> > > +
> > > +struct virtio_device;
> > > +
> > > +extern bool platform_forces_virtio_dma(struct virtio_device *vdev);
> > >  #endif	/* _ASM_DMA_MAPPING_H */
> > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > > index 06f0296..a2ec15a 100644
> > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > @@ -38,6 +38,7 @@
> > >  #include <linux/of.h>
> > >  #include <linux/iommu.h>
> > >  #include <linux/rculist.h>
> > > +#include <linux/virtio.h>
> > >  #include <asm/io.h>
> > >  #include <asm/prom.h>
> > >  #include <asm/rtas.h>
> > > @@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str)
> > >  __setup("multitce=", disable_multitce);
> > >  
> > >  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> > > +
> > > +bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > > +{
> > > +	/*
> > > +	 * On protected guest platforms, force virtio core to use DMA
> > > +	 * MAP API for all virtio devices. But there can also be some
> > > +	 * exceptions for individual devices like virtio balloon.
> > > +	 */
> > > +	return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
> > > +}
> > 
> > Isn't this kind of slow?  vring_use_dma_api is on
> > data path and supposed to be very fast.
> > 
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index 21d464a..47ea6c3 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -141,8 +141,18 @@ struct vring_virtqueue {
> > >   * unconditionally on data path.
> > >   */
> > >  
> > > +#ifndef platform_forces_virtio_dma
> > > +static inline bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > > +{
> > > +	return false;
> > > +}
> > > +#endif
> > > +
> > >  static bool vring_use_dma_api(struct virtio_device *vdev)
> > >  {
> > > +	if (platform_forces_virtio_dma(vdev))
> > > +		return true;
> > > +
> > >  	if (!virtio_has_iommu_quirk(vdev))
> > >  		return true;
> > >  
> > > -- 
> > > 2.9.3

^ permalink raw reply

* Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
From: Mathieu Malaterre @ 2018-06-04 12:06 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: qiang.zhao, kernel-janitors, linuxppc-dev, LKML, leoyang.li
In-Reply-To: <20180604115842.7c4vzge3igjbnblt@kili.mountain>

Where did the original go ?

https://patchwork.ozlabs.org/patch/868158/


On Mon, Jun 4, 2018 at 2:02 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> There is a copy and paste bug so we accidentally use the RX_ shift when
> we're in TX_ mode.
>
> Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analysis work.  Not tested.  This affects the success path, so
> we should probably test it.
>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index c646d8713861..681f7d4b7724 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 tdm_num)
>  {
>         u32 shift;
>
> -       shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
> +       shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : TX_SYNC_SHIFT_BASE;
>         shift -= tdm_num * 2;
>
>         return shift;

^ permalink raw reply

* Re: [PATCH 10/11] macintosh/via-pmu: Clean up interrupt statistics
From: Geert Uytterhoeven @ 2018-06-04 12:00 UTC (permalink / raw)
  To: Finn Thain
  Cc: Benjamin Herrenschmidt, Michael Schmitz, linuxppc-dev, linux-m68k,
	Linux Kernel Mailing List
In-Reply-To: <af2106d65c6b845c178bb2a0837e8554be2d1e8e.1527909627.git.fthain@telegraphics.com.au>

Hi Finn,

On Sat, Jun 2, 2018 at 5:27 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Replace an open-coded ffs() with the function call.
> Simplify an if-else cascade using a switch statement.
> Correct a typo and an indentation issue.
>
> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

A few minor nits below...

> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c

> @@ -1470,25 +1470,25 @@ pmu_handle_data(unsigned char *data, int len)
>                                 adb_input(data+1, len-1, 1);
>  #endif /* CONFIG_ADB */
>                 }
> -       }
> +               break;
>         /* Sound/brightness button pressed */
> -       else if ((1 << pirq) & PMU_INT_SNDBRT) {
> +       case PMU_INT_SNDBRT:
>  #ifdef CONFIG_PMAC_BACKLIGHT
>                 if (len == 3)
>                         pmac_backlight_set_legacy_brightness_pmu(data[1] >> 4);
>  #endif
> -       }
> +               break;

Please add a blank line after each "break" statement.

>         /* Tick interrupt */
> -       else if ((1 << pirq) & PMU_INT_TICK) {
> -               /* Environement or tick interrupt, query batteries */
> +       case PMU_INT_TICK:

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
From: Dan Carpenter @ 2018-06-04 11:58 UTC (permalink / raw)
  To: Qiang Zhao; +Cc: Li Yang, linuxppc-dev, linux-kernel, kernel-janitors

There is a copy and paste bug so we accidentally use the RX_ shift when
we're in TX_ mode.

Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static analysis work.  Not tested.  This affects the success path, so
we should probably test it.

diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
index c646d8713861..681f7d4b7724 100644
--- a/drivers/soc/fsl/qe/ucc.c
+++ b/drivers/soc/fsl/qe/ucc.c
@@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 tdm_num)
 {
 	u32 shift;
 
-	shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
+	shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : TX_SYNC_SHIFT_BASE;
 	shift -= tdm_num * 2;
 
 	return shift;

^ permalink raw reply related

* Re: [PATCH 08/11] macintosh/via-pmu: Replace via-pmu68k driver with via-pmu driver
From: Geert Uytterhoeven @ 2018-06-04 11:55 UTC (permalink / raw)
  To: Finn Thain
  Cc: Benjamin Herrenschmidt, Michael Schmitz, linuxppc-dev, linux-m68k,
	Linux Kernel Mailing List
In-Reply-To: <de8edb03dbd2fd25b53d0bdf9c6d65adf9aa0aa3.1527909627.git.fthain@telegraphics.com.au>

Hi Finn,

On Sat, Jun 2, 2018 at 5:27 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Now that the PowerMac via-pmu driver supports m68k PowerBooks,
> switch over to that driver and remove the via-pmu68k driver.

Thanks!

> Don't call pmu_shutdown() or pmu_restart() on early PowerBooks:
> the PMU device found in these PowerBooks isn't supported.

Shouldn't that be a separate patch?

> --- a/arch/m68k/mac/misc.c
> +++ b/arch/m68k/mac/misc.c

> @@ -477,9 +445,8 @@ void mac_poweroff(void)
>                    macintosh_config->adb_type == MAC_ADB_CUDA) {
>                 cuda_shutdown();
>  #endif
> -#ifdef CONFIG_ADB_PMU68K
> -       } else if (macintosh_config->adb_type == MAC_ADB_PB1
> -               || macintosh_config->adb_type == MAC_ADB_PB2) {
> +#ifdef CONFIG_ADB_PMU
> +       } else if (macintosh_config->adb_type == MAC_ADB_PB2) {
>                 pmu_shutdown();
>  #endif
>         }
> @@ -519,9 +486,8 @@ void mac_reset(void)
>                    macintosh_config->adb_type == MAC_ADB_CUDA) {
>                 cuda_restart();
>  #endif
> -#ifdef CONFIG_ADB_PMU68K
> -       } else if (macintosh_config->adb_type == MAC_ADB_PB1
> -               || macintosh_config->adb_type == MAC_ADB_PB2) {
> +#ifdef CONFIG_ADB_PMU
> +       } else if (macintosh_config->adb_type == MAC_ADB_PB2) {
>                 pmu_restart();
>  #endif
>         } else if (CPU_IS_030) {

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 03/11] macintosh/via-pmu: Don't clear shift register interrupt flag twice
From: Geert Uytterhoeven @ 2018-06-04 11:48 UTC (permalink / raw)
  To: Finn Thain
  Cc: Benjamin Herrenschmidt, Michael Schmitz, linuxppc-dev, linux-m68k,
	Linux Kernel Mailing List
In-Reply-To: <8597ac466215f573d8bea40e74fdec2bf72fabbb.1527909627.git.fthain@telegraphics.com.au>

On Sat, Jun 2, 2018 at 5:27 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Clearing the interrupt flag twice in succession creates a theoretical
> race condition. Fix this.

I would add that the caller of pmu_sr_intr() has already cleared the flag,
so the casual reviewer doesn't have to hunt for it.

> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 02/11] macintosh/via-pmu: Add missing mmio accessors
From: Geert Uytterhoeven @ 2018-06-04 11:46 UTC (permalink / raw)
  To: Finn Thain
  Cc: Benjamin Herrenschmidt, Michael Schmitz, linuxppc-dev, linux-m68k,
	Linux Kernel Mailing List
In-Reply-To: <7ce9a8c3996506ae6f9ab16b3cf73c287e205a89.1527909627.git.fthain@telegraphics.com.au>

On Sat, Jun 2, 2018 at 5:27 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Add missing in_8() accessors to init_pmu() and pmu_sr_intr().
>
> This fixes several sparse warnings:
> drivers/macintosh/via-pmu.c:536:29: warning: dereference of noderef expression
> drivers/macintosh/via-pmu.c:537:33: warning: dereference of noderef expression
> drivers/macintosh/via-pmu.c:1455:17: warning: dereference of noderef expression
> drivers/macintosh/via-pmu.c:1456:69: warning: dereference of noderef expression
>
> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2] cpuidle/powernv : Add Description for cpuidle state
From: Akshay Adiga @ 2018-06-04 11:45 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Abhishek Goel, rjw, daniel.lezcano, paulus, mpe, linux-pm,
	linuxppc-dev, linux-kernel, stewart
In-Reply-To: <0a28154deb468669a59c120a90d4aedc1e1e705b.camel@kernel.crashing.org>

On Mon, Jun 04, 2018 at 07:04:14PM +1000, Benjamin Herrenschmidt wrote:
> Is this a new property ? I'm not fan of adding yet another of those
> silly arrays.
> 
> I would say this is the right time now to switch over to a node per
> state instead, as we discussed with Vaidy.

I posted  the node based device tree here :
skiboot patch :  https://patchwork.ozlabs.org/patch/923120/
kernel patch : https://lkml.org/lkml/2018/5/30/1146

Do you have any inputs for this design ?

> 
> Additionally, while doing that, we can provide the versioning mechanism
> I proposed so we can deal with state specific issues and erratas.
> 
> Cheers,
> Ben.
> 

^ permalink raw reply

* Re: [PATCH 01/11] macintosh/via-pmu: Fix section mismatch warning
From: Geert Uytterhoeven @ 2018-06-04 11:44 UTC (permalink / raw)
  To: Finn Thain
  Cc: Benjamin Herrenschmidt, Michael Schmitz, linuxppc-dev, linux-m68k,
	Linux Kernel Mailing List
In-Reply-To: <f32f4e3a0fd736152f768b7a00bfc731daba1629.1527909627.git.fthain@telegraphics.com.au>

Hi Finn,

On Sat, Jun 2, 2018 at 5:27 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> The pmu_init() function has the __init qualifier, but the ops struct
> that holds a pointer to it does not. This causes a build warning.
> The driver works fine because the pointer is only dereferenced early.
>
> The function is so small that there's negligible benefit from using
> the __init qualifier. Remove it to fix the warning, consistent with
> the other ADB drivers.

Some other ADB subdriver .init() and .probe() functions aren't that small.
But with the current scheme using adb_drivers_list[], they cannot be __init.
Probably the long term fix is to change the ADB subsystem from the
centralized approach of letting adb_init() call all subdrivers, to making the
subdrivers platform drivers registering with the ADB core.

> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Anyway:
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] cpuidle:powernv: Make the snooze timeout dynamic.
From: Michael Ellerman @ 2018-06-04 11:27 UTC (permalink / raw)
  To: Gautham R. Shenoy, Rafael J. Wysocki, Daniel Lezcano,
	Stewart Smith, Michael Neuling, Vaidyanathan Srinivasan,
	Shilpasri G Bhat, Akshay Adiga, Nicholas Piggin
  Cc: linuxppc-dev, linux-kernel, linux-pm, Gautham R. Shenoy
In-Reply-To: <1527768909-32637-1-git-send-email-ego@linux.vnet.ibm.com>

"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:

> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> The commit 78eaa10f027c ("cpuidle: powernv/pseries: Auto-promotion of
> snooze to deeper idle state") introduced a timeout for the snooze idle
> state so that it could be eventually be promoted to a deeper idle
> state. The snooze timeout value is static and set to the target
> residency of the next idle state, which would train the cpuidle
> governor to pick the next idle state eventually.
>
> The unfortunate side-effect of this is that if the next idle state(s)
> is disabled, the CPU will forever remain in snooze, despite the fact
> that the system is completely idle, and other deeper idle states are
> available.

That sounds like a bug, I'll add?

Fixes: 78eaa10f027c ("cpuidle: powernv/pseries: Auto-promotion of snooze to deeper idle state")
Cc: stable@vger.kernel.org # v4.2+

cheers

^ permalink raw reply

* Re: linux-next: Signed-off-by missing for commit in the powerpc tree
From: Michael Ellerman @ 2018-06-04 10:39 UTC (permalink / raw)
  To: Stephen Rothwell, Benjamin Herrenschmidt, PowerPC
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
	Michal Suchanek
In-Reply-To: <20180604042626.286a9469@canb.auug.org.au>

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi all,
>
> Commit
>
>   cb3d6759a93c ("powerpc/64s: Enable barrier_nospec based on firmware settings")
>
> is missing a Signed-off-by from its author.

Sorry my fault.

cheers

^ permalink raw reply

* Re: pkeys on POWER: Access rights not reset on execve
From: Florian Weimer @ 2018-06-04 10:12 UTC (permalink / raw)
  To: Ram Pai; +Cc: Andy Lutomirski, Linux-MM, linuxppc-dev, Dave Hansen
In-Reply-To: <20180603201832.GA10109@ram.oc3035372033.ibm.com>

On 06/03/2018 10:18 PM, Ram Pai wrote:
> On Mon, May 21, 2018 at 01:29:11PM +0200, Florian Weimer wrote:
>> On 05/20/2018 09:11 PM, Ram Pai wrote:
>>> Florian,
>>>
>>> 	Does the following patch fix the problem for you?  Just like x86
>>> 	I am enabling all keys in the UAMOR register during
>>> 	initialization itself. Hence any key created by any thread at
>>> 	any time, will get activated on all threads. So any thread
>>> 	can change the permission on that key. Smoke tested it
>>> 	with your test program.
>>
>> I think this goes in the right direction, but the AMR value after
>> fork is still strange:
>>
>> AMR (PID 34912): 0x0000000000000000
>> AMR after fork (PID 34913): 0x0000000000000000
>> AMR (PID 34913): 0x0000000000000000
>> Allocated key in subprocess (PID 34913): 2
>> Allocated key (PID 34912): 2
>> Setting AMR: 0xffffffffffffffff
>> New AMR value (PID 34912): 0x0fffffffffffffff
>> About to call execl (PID 34912) ...
>> AMR (PID 34912): 0x0fffffffffffffff
>> AMR after fork (PID 34914): 0x0000000000000003
>> AMR (PID 34914): 0x0000000000000003
>> Allocated key in subprocess (PID 34914): 2
>> Allocated key (PID 34912): 2
>> Setting AMR: 0xffffffffffffffff
>> New AMR value (PID 34912): 0x0fffffffffffffff
>>
>> I mean this line:
>>
>> AMR after fork (PID 34914): 0x0000000000000003
>>
>> Shouldn't it be the same as in the parent process?
> 
> Fixed it. Please try this patch. If it all works to your satisfaction, I
> will clean it up further and send to Michael Ellermen(ppc maintainer).
> 
> 
> commit 51f4208ed5baeab1edb9b0f8b68d7144449b3527
> Author: Ram Pai <linuxram@us.ibm.com>
> Date:   Sun Jun 3 14:44:32 2018 -0500
> 
>      Fix for the fork bug.
>      
>      Signed-off-by: Ram Pai <linuxram@us.ibm.com>

Is this on top of the previous patch, or a separate fix?

Thanks,
Florian

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Benjamin Herrenschmidt @ 2018-06-04  9:48 UTC (permalink / raw)
  To: David Gibson
  Cc: Michael S. Tsirkin, Anshuman Khandual, virtualization,
	linux-kernel, linuxppc-dev, aik, robh, joe, elfring, jasowang,
	mpe, hch
In-Reply-To: <20180604085742.GQ4251@umbus>

On Mon, 2018-06-04 at 18:57 +1000, David Gibson wrote:
> 
> > - First qemu doesn't know that the guest will switch to "secure mode"
> > in advance. There is no difference between a normal and a secure
> > partition until the partition does the magic UV call to "enter secure
> > mode" and qemu doesn't see any of it. So who can set the flag here ?
> 
> This seems weird to me.  As a rule HV calls should go through qemu -
> or be allowed to go directly to KVM *by* qemu.

It's not an HV call, it's a UV call, qemu won't see it, qemu isn't
trusted. Now the UV *will* reflect that to the HV via some synthetized
HV calls, and we *could* have those do a pass by qemu, however, so far,
our entire design doesn't rely on *any* qemu knowledge whatsoever and
it would be sad to add it just for that purpose.

Additionally, this is rather orthogonal, see my other email, the
problem we are trying to solve is *not* a qemu problem and it doesn't
make sense to leak that into qemu.

>   We generally reserve
> the latter for hot path things.  Since this isn't a hot path, having
> the call handled directly by the kernel seems wrong.
>
> Unless a "UV call" is something different I don't know about.

Yes, a UV call goes to the Ultravisor, not the Hypervisor. The
Hypervisor isn't trusted.

> > - Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
> > vhost) go through the emulated MMIO for every access to the guest,
> > which adds additional overhead.
> 
Ben.

^ permalink raw reply

* Re: [PATCH v2 07/13] powerpc/eeh: Clean up pci_ers_result handling
From: Michael Ellerman @ 2018-06-04  9:43 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: linuxppc-dev
In-Reply-To: <20180604012833.GA6139@tungsten.ozlabs.ibm.com>

Sam Bobroff <sbobroff@linux.ibm.com> writes:

> On Sat, Jun 02, 2018 at 01:40:46AM +1000, Michael Ellerman wrote:
>> Sam Bobroff <sbobroff@linux.ibm.com> writes:
>> 
>> > As EEH event handling progresses, a cumulative result of type
>> > pci_ers_result is built up by (some of) the eeh_report_*() functions
>> > using either:
>> > 	if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
>> > 	if (*res == PCI_ERS_RESULT_NONE) *res = rc;
>> > or:
>> > 	if ((*res == PCI_ERS_RESULT_NONE) ||
>> > 	    (*res == PCI_ERS_RESULT_RECOVERED)) *res = rc;
>> > 	if (*res == PCI_ERS_RESULT_DISCONNECT &&
>> > 	    rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
>> > (Where *res is the accumulator.)
>> >
>> > However, the intent is not immediately clear and the result in some
>> > situations is order dependent.
>> >
>> > Address this by assigning a priority to each result value, and always
>> > merging to the highest priority. This renders the intent clear, and
>> > provides a stable value for all orderings.
>> >
>> > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
>> > ---
>> > ====== v1 -> v2: ======
>> >
>> > * Added the value, and missing newline, to some WARN()s.
>> > * Improved name of merge_result() to pci_ers_merge_result().
>> > * Adjusted the result priorities so that unknown doesn't overlap with _NONE.
>> 
>> These === markers seem to have confused patchwork, they ended up in the
>> patch, and then git put them in the changelog.
>> 
>> http://patchwork.ozlabs.org/patch/920194/
>> 
>> The usual format is just something like:
>> 
>> v2 - Added the value, and missing newline, to some WARN()s.
>>    - Improved name of merge_result() to pci_ers_merge_result().
>>    - Adjusted the result priorities so that unknown doesn't overlap with _NONE.
>> 
>> cheers
>
> Oh! I'll change it!

Thanks.

cheers

^ permalink raw reply


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