* Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
From: Jesper Dangaard Brouer @ 2021-04-16 17:08 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Arnd Bergmann, Grygorii Strashko, netdev@vger.kernel.org,
Ilias Apalodimas, linux-kernel@vger.kernel.org,
linux-mips@vger.kernel.org, linux-mm@kvack.org, David Laight,
brouer, Matteo Croce, linuxppc-dev@lists.ozlabs.org,
Christoph Hellwig, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20210416152755.GL2531743@casper.infradead.org>
On Fri, 16 Apr 2021 16:27:55 +0100
Matthew Wilcox <willy@infradead.org> wrote:
> On Thu, Apr 15, 2021 at 08:08:32PM +0200, Jesper Dangaard Brouer wrote:
> > See below patch. Where I swap32 the dma address to satisfy
> > page->compound having bit zero cleared. (It is the simplest fix I could
> > come up with).
>
> I think this is slightly simpler, and as a bonus code that assumes the
> old layout won't compile.
This is clever, I like it! When reading the code one just have to
remember 'unsigned long' size difference between 64-bit vs 32-bit.
And I assume compiler can optimize the sizeof check out then doable.
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 6613b26a8894..5aacc1c10a45 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -97,10 +97,10 @@ struct page {
> };
> struct { /* page_pool used by netstack */
> /**
> - * @dma_addr: might require a 64-bit value even on
> + * @dma_addr: might require a 64-bit value on
> * 32-bit architectures.
> */
> - dma_addr_t dma_addr;
> + unsigned long dma_addr[2];
> };
> struct { /* slab, slob and slub */
> union {
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index b5b195305346..db7c7020746a 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -198,7 +198,17 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>
> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> {
> - return page->dma_addr;
> + dma_addr_t ret = page->dma_addr[0];
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + ret |= (dma_addr_t)page->dma_addr[1] << 32;
> + return ret;
> +}
> +
> +static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
> +{
> + page->dma_addr[0] = addr;
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + page->dma_addr[1] = addr >> 32;
> }
>
> static inline bool is_page_pool_compiled_in(void)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index ad8b0707af04..f014fd8c19a6 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -174,8 +174,10 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool,
> struct page *page,
> unsigned int dma_sync_size)
> {
> + dma_addr_t dma_addr = page_pool_get_dma_addr(page);
> +
> dma_sync_size = min(dma_sync_size, pool->p.max_len);
> - dma_sync_single_range_for_device(pool->p.dev, page->dma_addr,
> + dma_sync_single_range_for_device(pool->p.dev, dma_addr,
> pool->p.offset, dma_sync_size,
> pool->p.dma_dir);
> }
> @@ -226,7 +228,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
> put_page(page);
> return NULL;
> }
> - page->dma_addr = dma;
> + page_pool_set_dma_addr(page, dma);
>
> if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
> page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
> @@ -294,13 +296,13 @@ void page_pool_release_page(struct page_pool *pool, struct page *page)
> */
> goto skip_dma_unmap;
>
> - dma = page->dma_addr;
> + dma = page_pool_get_dma_addr(page);
>
> - /* When page is unmapped, it cannot be returned our pool */
> + /* When page is unmapped, it cannot be returned to our pool */
> dma_unmap_page_attrs(pool->p.dev, dma,
> PAGE_SIZE << pool->p.order, pool->p.dma_dir,
> DMA_ATTR_SKIP_CPU_SYNC);
> - page->dma_addr = 0;
> + page_pool_set_dma_addr(page, 0);
> skip_dma_unmap:
> /* This may be the last page returned, releasing the pool, so
> * it is not safe to reference pool afterwards.
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next 0/2] net: gianfar: Drop GFAR_MQ_POLLING support
From: Claudiu Manoil @ 2021-04-16 17:11 UTC (permalink / raw)
To: netdev
Cc: devicetree, Vladimir Oltean, Rob Herring, Jakub Kicinski,
linuxppc-dev, David S . Miller
Drop long time obsolete "per NAPI multi-queue" support in gianfar,
and related (and undocumented) device tree properties.
Claudiu Manoil (2):
gianfar: Drop GFAR_MQ_POLLING support
powerpc: dts: fsl: Drop obsolete fsl,rx-bit-map and fsl,tx-bit-map
properties
arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi | 4 -
arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi | 4 -
arch/powerpc/boot/dts/fsl/c293si-post.dtsi | 4 -
arch/powerpc/boot/dts/fsl/p1010si-post.dtsi | 21 ---
drivers/net/ethernet/freescale/gianfar.c | 170 ++----------------
drivers/net/ethernet/freescale/gianfar.h | 17 --
6 files changed, 11 insertions(+), 209 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH net-next 1/2] gianfar: Drop GFAR_MQ_POLLING support
From: Claudiu Manoil @ 2021-04-16 17:11 UTC (permalink / raw)
To: netdev
Cc: devicetree, Vladimir Oltean, Rob Herring, Jakub Kicinski,
linuxppc-dev, David S . Miller
In-Reply-To: <20210416171123.22969-1-claudiu.manoil@nxp.com>
Gianfar used to enable all 8 Rx queues (DMA rings) per
ethernet device, even though the controller can only
support 2 interrupt lines at most. This meant that
multiple Rx queues would have to be grouped per NAPI poll
routine, and the CPU would have to split the budget and
service them in a round robin manner. The overhead of
this scheme proved to outweight the potential benefits.
The alternative was to introduce the "Single Queue" polling
mode, supporting one Rx queue per NAPI, which became the
default packet processing option and helped improve the
performance of the driver.
MQ_POLLING also relies on undocumeted device tree properties
to specify how to map the 8 Rx and Tx queues to a given
interrupt line (aka "interrupt group"). Using module parameters
to enable this mode wasn't an option either. Long story short,
MQ_POLLING became obsolete, now it is just dead code, and no
one asked for it so far.
For the Tx queues, multi-queue support (more than 1 Tx queue
per CPU) could be revisited by adding tc MQPRIO support, but
again, one has to consider that there are only 2 interrupt lines.
So the NAPI poll routine would have to service multiple Tx rings.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
drivers/net/ethernet/freescale/gianfar.c | 170 ++---------------------
drivers/net/ethernet/freescale/gianfar.h | 17 ---
2 files changed, 11 insertions(+), 176 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 3ec4d9fddd52..4e4c62d4061e 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -175,10 +175,7 @@ static void gfar_mac_rx_config(struct gfar_private *priv)
if (priv->rx_filer_enable) {
rctrl |= RCTRL_FILREN | RCTRL_PRSDEP_INIT;
/* Program the RIR0 reg with the required distribution */
- if (priv->poll_mode == GFAR_SQ_POLLING)
- gfar_write(®s->rir0, DEFAULT_2RXQ_RIR0);
- else /* GFAR_MQ_POLLING */
- gfar_write(®s->rir0, DEFAULT_8RXQ_RIR0);
+ gfar_write(®s->rir0, DEFAULT_2RXQ_RIR0);
}
/* Restore PROMISC mode */
@@ -521,29 +518,9 @@ static int gfar_parse_group(struct device_node *np,
grp->priv = priv;
spin_lock_init(&grp->grplock);
if (priv->mode == MQ_MG_MODE) {
- u32 rxq_mask, txq_mask;
- int ret;
-
+ /* One Q per interrupt group: Q0 to G0, Q1 to G1 */
grp->rx_bit_map = (DEFAULT_MAPPING >> priv->num_grps);
grp->tx_bit_map = (DEFAULT_MAPPING >> priv->num_grps);
-
- ret = of_property_read_u32(np, "fsl,rx-bit-map", &rxq_mask);
- if (!ret) {
- grp->rx_bit_map = rxq_mask ?
- rxq_mask : (DEFAULT_MAPPING >> priv->num_grps);
- }
-
- ret = of_property_read_u32(np, "fsl,tx-bit-map", &txq_mask);
- if (!ret) {
- grp->tx_bit_map = txq_mask ?
- txq_mask : (DEFAULT_MAPPING >> priv->num_grps);
- }
-
- if (priv->poll_mode == GFAR_SQ_POLLING) {
- /* One Q per interrupt group: Q0 to G0, Q1 to G1 */
- grp->rx_bit_map = (DEFAULT_MAPPING >> priv->num_grps);
- grp->tx_bit_map = (DEFAULT_MAPPING >> priv->num_grps);
- }
} else {
grp->rx_bit_map = 0xFF;
grp->tx_bit_map = 0xFF;
@@ -650,18 +627,15 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
u32 stash_len = 0;
u32 stash_idx = 0;
unsigned int num_tx_qs, num_rx_qs;
- unsigned short mode, poll_mode;
+ unsigned short mode;
if (!np)
return -ENODEV;
- if (of_device_is_compatible(np, "fsl,etsec2")) {
+ if (of_device_is_compatible(np, "fsl,etsec2"))
mode = MQ_MG_MODE;
- poll_mode = GFAR_SQ_POLLING;
- } else {
+ else
mode = SQ_SG_MODE;
- poll_mode = GFAR_SQ_POLLING;
- }
if (mode == SQ_SG_MODE) {
num_tx_qs = 1;
@@ -677,22 +651,8 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
return -EINVAL;
}
- if (poll_mode == GFAR_SQ_POLLING) {
- num_tx_qs = num_grps; /* one txq per int group */
- num_rx_qs = num_grps; /* one rxq per int group */
- } else { /* GFAR_MQ_POLLING */
- u32 tx_queues, rx_queues;
- int ret;
-
- /* parse the num of HW tx and rx queues */
- ret = of_property_read_u32(np, "fsl,num_tx_queues",
- &tx_queues);
- num_tx_qs = ret ? 1 : tx_queues;
-
- ret = of_property_read_u32(np, "fsl,num_rx_queues",
- &rx_queues);
- num_rx_qs = ret ? 1 : rx_queues;
- }
+ num_tx_qs = num_grps; /* one txq per int group */
+ num_rx_qs = num_grps; /* one rxq per int group */
}
if (num_tx_qs > MAX_TX_QS) {
@@ -718,7 +678,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
priv->ndev = dev;
priv->mode = mode;
- priv->poll_mode = poll_mode;
priv->num_tx_queues = num_tx_qs;
netif_set_real_num_rx_queues(dev, num_rx_qs);
@@ -2695,106 +2654,6 @@ static int gfar_poll_tx_sq(struct napi_struct *napi, int budget)
return 0;
}
-static int gfar_poll_rx(struct napi_struct *napi, int budget)
-{
- struct gfar_priv_grp *gfargrp =
- container_of(napi, struct gfar_priv_grp, napi_rx);
- struct gfar_private *priv = gfargrp->priv;
- struct gfar __iomem *regs = gfargrp->regs;
- struct gfar_priv_rx_q *rx_queue = NULL;
- int work_done = 0, work_done_per_q = 0;
- int i, budget_per_q = 0;
- unsigned long rstat_rxf;
- int num_act_queues;
-
- /* Clear IEVENT, so interrupts aren't called again
- * because of the packets that have already arrived
- */
- gfar_write(®s->ievent, IEVENT_RX_MASK);
-
- rstat_rxf = gfar_read(®s->rstat) & RSTAT_RXF_MASK;
-
- num_act_queues = bitmap_weight(&rstat_rxf, MAX_RX_QS);
- if (num_act_queues)
- budget_per_q = budget/num_act_queues;
-
- for_each_set_bit(i, &gfargrp->rx_bit_map, priv->num_rx_queues) {
- /* skip queue if not active */
- if (!(rstat_rxf & (RSTAT_CLEAR_RXF0 >> i)))
- continue;
-
- rx_queue = priv->rx_queue[i];
- work_done_per_q =
- gfar_clean_rx_ring(rx_queue, budget_per_q);
- work_done += work_done_per_q;
-
- /* finished processing this queue */
- if (work_done_per_q < budget_per_q) {
- /* clear active queue hw indication */
- gfar_write(®s->rstat,
- RSTAT_CLEAR_RXF0 >> i);
- num_act_queues--;
-
- if (!num_act_queues)
- break;
- }
- }
-
- if (!num_act_queues) {
- u32 imask;
- napi_complete_done(napi, work_done);
-
- /* Clear the halt bit in RSTAT */
- gfar_write(®s->rstat, gfargrp->rstat);
-
- spin_lock_irq(&gfargrp->grplock);
- imask = gfar_read(®s->imask);
- imask |= IMASK_RX_DEFAULT;
- gfar_write(®s->imask, imask);
- spin_unlock_irq(&gfargrp->grplock);
- }
-
- return work_done;
-}
-
-static int gfar_poll_tx(struct napi_struct *napi, int budget)
-{
- struct gfar_priv_grp *gfargrp =
- container_of(napi, struct gfar_priv_grp, napi_tx);
- struct gfar_private *priv = gfargrp->priv;
- struct gfar __iomem *regs = gfargrp->regs;
- struct gfar_priv_tx_q *tx_queue = NULL;
- int has_tx_work = 0;
- int i;
-
- /* Clear IEVENT, so interrupts aren't called again
- * because of the packets that have already arrived
- */
- gfar_write(®s->ievent, IEVENT_TX_MASK);
-
- for_each_set_bit(i, &gfargrp->tx_bit_map, priv->num_tx_queues) {
- tx_queue = priv->tx_queue[i];
- /* run Tx cleanup to completion */
- if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) {
- gfar_clean_tx_ring(tx_queue);
- has_tx_work = 1;
- }
- }
-
- if (!has_tx_work) {
- u32 imask;
- napi_complete(napi);
-
- spin_lock_irq(&gfargrp->grplock);
- imask = gfar_read(®s->imask);
- imask |= IMASK_TX_DEFAULT;
- gfar_write(®s->imask, imask);
- spin_unlock_irq(&gfargrp->grplock);
- }
-
- return 0;
-}
-
/* GFAR error interrupt handler */
static irqreturn_t gfar_error(int irq, void *grp_id)
{
@@ -3352,17 +3211,10 @@ static int gfar_probe(struct platform_device *ofdev)
/* Register for napi ...We are registering NAPI for each grp */
for (i = 0; i < priv->num_grps; i++) {
- if (priv->poll_mode == GFAR_SQ_POLLING) {
- netif_napi_add(dev, &priv->gfargrp[i].napi_rx,
- gfar_poll_rx_sq, GFAR_DEV_WEIGHT);
- netif_tx_napi_add(dev, &priv->gfargrp[i].napi_tx,
- gfar_poll_tx_sq, 2);
- } else {
- netif_napi_add(dev, &priv->gfargrp[i].napi_rx,
- gfar_poll_rx, GFAR_DEV_WEIGHT);
- netif_tx_napi_add(dev, &priv->gfargrp[i].napi_tx,
- gfar_poll_tx, 2);
- }
+ netif_napi_add(dev, &priv->gfargrp[i].napi_rx,
+ gfar_poll_rx_sq, GFAR_DEV_WEIGHT);
+ netif_tx_napi_add(dev, &priv->gfargrp[i].napi_tx,
+ gfar_poll_tx_sq, 2);
}
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_CSUM) {
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 8ced783f5302..5ea47df93e5e 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -909,22 +909,6 @@ enum {
MQ_MG_MODE
};
-/* GFAR_SQ_POLLING: Single Queue NAPI polling mode
- * The driver supports a single pair of RX/Tx queues
- * per interrupt group (Rx/Tx int line). MQ_MG mode
- * devices have 2 interrupt groups, so the device will
- * have a total of 2 Tx and 2 Rx queues in this case.
- * GFAR_MQ_POLLING: Multi Queue NAPI polling mode
- * The driver supports all the 8 Rx and Tx HW queues
- * each queue mapped by the Device Tree to one of
- * the 2 interrupt groups. This mode implies significant
- * processing overhead (CPU and controller level).
- */
-enum gfar_poll_mode {
- GFAR_SQ_POLLING = 0,
- GFAR_MQ_POLLING
-};
-
/*
* Per TX queue stats
*/
@@ -1105,7 +1089,6 @@ struct gfar_private {
unsigned long state;
unsigned short mode;
- unsigned short poll_mode;
unsigned int num_tx_queues;
unsigned int num_rx_queues;
unsigned int num_grps;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next 2/2] powerpc: dts: fsl: Drop obsolete fsl, rx-bit-map and fsl, tx-bit-map properties
From: Claudiu Manoil @ 2021-04-16 17:11 UTC (permalink / raw)
To: netdev
Cc: devicetree, Vladimir Oltean, Rob Herring, Jakub Kicinski,
linuxppc-dev, David S . Miller
In-Reply-To: <20210416171123.22969-1-claudiu.manoil@nxp.com>
These are very old properties that were used by the "gianfar" ethernet
driver. They don't have documented bindings and are obsolete.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi | 4 ----
arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi | 4 ----
arch/powerpc/boot/dts/fsl/c293si-post.dtsi | 4 ----
arch/powerpc/boot/dts/fsl/p1010si-post.dtsi | 21 -------------------
4 files changed, 33 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi b/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
index 0c0efa94cfb4..2a677fd323eb 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
@@ -170,8 +170,6 @@ timer@41100 {
/include/ "pq3-etsec2-0.dtsi"
enet0: ethernet@b0000 {
queue-group@b0000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
interrupts = <26 2 0 0 27 2 0 0 28 2 0 0>;
};
};
@@ -179,8 +177,6 @@ queue-group@b0000 {
/include/ "pq3-etsec2-1.dtsi"
enet1: ethernet@b1000 {
queue-group@b1000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
interrupts = <33 2 0 0 34 2 0 0 35 2 0 0>;
};
};
diff --git a/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi b/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
index b5f071574e83..b8e0edd1ac69 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
@@ -190,8 +190,6 @@ sec_jr3: jr@4000 {
/include/ "pq3-etsec2-0.dtsi"
enet0: ethernet@b0000 {
queue-group@b0000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
interrupts = <26 2 0 0 27 2 0 0 28 2 0 0>;
};
};
@@ -199,8 +197,6 @@ queue-group@b0000 {
/include/ "pq3-etsec2-1.dtsi"
enet1: ethernet@b1000 {
queue-group@b1000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
interrupts = <33 2 0 0 34 2 0 0 35 2 0 0>;
};
};
diff --git a/arch/powerpc/boot/dts/fsl/c293si-post.dtsi b/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
index bd208320bff5..bec0fc36849d 100644
--- a/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
@@ -171,8 +171,6 @@ jr@2000{
enet0: ethernet@b0000 {
queue-group@b0000 {
reg = <0x10000 0x1000>;
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
};
};
@@ -180,8 +178,6 @@ queue-group@b0000 {
enet1: ethernet@b1000 {
queue-group@b1000 {
reg = <0x11000 0x1000>;
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
};
};
diff --git a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
index 1b4aafc1f6a2..c2717f31925a 100644
--- a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
@@ -172,29 +172,8 @@ sdhc@2e000 {
/include/ "pq3-mpic-timer-B.dtsi"
/include/ "pq3-etsec2-0.dtsi"
- enet0: ethernet@b0000 {
- queue-group@b0000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
- };
- };
-
/include/ "pq3-etsec2-1.dtsi"
- enet1: ethernet@b1000 {
- queue-group@b1000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
- };
- };
-
/include/ "pq3-etsec2-2.dtsi"
- enet2: ethernet@b2000 {
- queue-group@b2000 {
- fsl,rx-bit-map = <0xff>;
- fsl,tx-bit-map = <0xff>;
- };
-
- };
global-utilities@e0000 {
compatible = "fsl,p1010-guts";
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/kexec_file: use current CPU info while setting up FDT
From: Hari Bathini @ 2021-04-16 18:08 UTC (permalink / raw)
To: Sourabh Jain, mpe; +Cc: mahesh, bauerman, linuxppc-dev
In-Reply-To: <20210416124658.718860-1-sourabhjain@linux.ibm.com>
On 16/04/21 6:16 pm, Sourabh Jain wrote:
> kexec_file_load uses initial_boot_params in setting up the device-tree
> for the kernel to be loaded. Though initial_boot_params holds info
> about CPUs at the time of boot, it doesn't account for hot added CPUs.
>
> So, kexec'ing with kexec_file_load syscall would leave the kexec'ed
> kernel with inaccurate CPU info. Also, if kdump kernel is loaded with
> kexec_file_load syscall and the system crashes on a hot added CPU,
> capture kernel hangs failing to identify the boot CPU.
>
> Kernel panic - not syncing: sysrq triggered crash
> CPU: 24 PID: 6065 Comm: echo Kdump: loaded Not tainted 5.12.0-rc5upstream #54
> Call Trace:
> [c0000000e590fac0] [c0000000007b2400] dump_stack+0xc4/0x114 (unreliable)
> [c0000000e590fb00] [c000000000145290] panic+0x16c/0x41c
> [c0000000e590fba0] [c0000000008892e0] sysrq_handle_crash+0x30/0x40
> [c0000000e590fc00] [c000000000889cdc] __handle_sysrq+0xcc/0x1f0
> [c0000000e590fca0] [c00000000088a538] write_sysrq_trigger+0xd8/0x178
> [c0000000e590fce0] [c0000000005e9b7c] proc_reg_write+0x10c/0x1b0
> [c0000000e590fd10] [c0000000004f26d0] vfs_write+0xf0/0x330
> [c0000000e590fd60] [c0000000004f2aec] ksys_write+0x7c/0x140
> [c0000000e590fdb0] [c000000000031ee0] system_call_exception+0x150/0x290
> [c0000000e590fe10] [c00000000000ca5c] system_call_common+0xec/0x278
> --- interrupt: c00 at 0x7fff905b9664
> NIP: 00007fff905b9664 LR: 00007fff905320c4 CTR: 0000000000000000
> REGS: c0000000e590fe80 TRAP: 0c00 Not tainted (5.12.0-rc5upstream)
> MSR: 800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 28000242
> XER: 00000000
> IRQMASK: 0
> GPR00: 0000000000000004 00007ffff5fedf30 00007fff906a7300 0000000000000001
> GPR04: 000001002a7355b0 0000000000000002 0000000000000001 00007ffff5fef616
> GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000000
> GPR12: 0000000000000000 00007fff9073a160 0000000000000000 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 00007fff906a4ee0 0000000000000002 0000000000000001
> GPR24: 00007fff906a0898 0000000000000000 0000000000000002 000001002a7355b0
> GPR28: 0000000000000002 00007fff906a1790 000001002a7355b0 0000000000000002
> NIP [00007fff905b9664] 0x7fff905b9664
> LR [00007fff905320c4] 0x7fff905320c4
> --- interrupt: c00
>
> To avoid this from happening, extract current CPU info from of_root
> device node and use it for setting up the fdt in kexec_file_load case.
>
> Fixes: 6ecd0163d360 ("powerpc/kexec_file: Add appropriate regions for memory reserve map")
Missed marking stable@vger.kernel.org on Cc for this fix..
> +int add_node_prop(void *fdt, int node_offset, const struct device_node *np)
> +{
<SNIP>
> +int update_cpus_node(void *fdt)
> +{
I think the above two new functions should be marked 'static'...
Thanks
Hari
^ permalink raw reply
* Re: [PATCH v1 5/7] KVM: PPC: Book3S 64: Move interrupt early register setup to KVM
From: Fabiano Rosas @ 2021-04-16 18:25 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412075103.1533302-6-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Like the earlier patch for hcalls, KVM interrupt entry requires a
> different calling convention than the Linux interrupt handlers
> set up. Move the code that converts from one to the other into KVM.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kernel/exceptions-64s.S | 131 +++++----------------------
> arch/powerpc/kvm/book3s_64_entry.S | 50 +++++++++-
> 2 files changed, 71 insertions(+), 110 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 1bfd0d7af09e..cd1731642b12 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -187,7 +187,6 @@ do_define_int n
> .endif
> .endm
>
> -#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> /*
> * All interrupts which set HSRR registers, as well as SRESET and MCE and
> * syscall when invoked with "sc 1" switch to MSR[HV]=1 (HVMODE) to be taken,
> @@ -220,54 +219,25 @@ do_define_int n
> * to KVM to handle.
> */
>
> -.macro KVMTEST name
> +.macro KVMTEST name handler
> +#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> lbz r10,HSTATE_IN_GUEST(r13)
> cmpwi r10,0
> - bne \name\()_kvm
> -.endm
> -
> -.macro GEN_KVM name
> - .balign IFETCH_ALIGN_BYTES
> -\name\()_kvm:
> -
> -BEGIN_FTR_SECTION
> - ld r10,IAREA+EX_CFAR(r13)
> - std r10,HSTATE_CFAR(r13)
> -END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
> -
> - ld r10,IAREA+EX_CTR(r13)
> - mtctr r10
> -BEGIN_FTR_SECTION
> - ld r10,IAREA+EX_PPR(r13)
> - std r10,HSTATE_PPR(r13)
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> - ld r11,IAREA+EX_R11(r13)
> - ld r12,IAREA+EX_R12(r13)
> - std r12,HSTATE_SCRATCH0(r13)
> - sldi r12,r9,32
> - ld r9,IAREA+EX_R9(r13)
> - ld r10,IAREA+EX_R10(r13)
> /* HSRR variants have the 0x2 bit added to their trap number */
> .if IHSRR_IF_HVMODE
> BEGIN_FTR_SECTION
> - ori r12,r12,(IVEC + 0x2)
> + li r10,(IVEC + 0x2)
> FTR_SECTION_ELSE
> - ori r12,r12,(IVEC)
> + li r10,(IVEC)
> ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
> .elseif IHSRR
> - ori r12,r12,(IVEC+ 0x2)
> + li r10,(IVEC + 0x2)
> .else
> - ori r12,r12,(IVEC)
> + li r10,(IVEC)
> .endif
> - b kvmppc_interrupt
> -.endm
> -
> -#else
> -.macro KVMTEST name
> -.endm
> -.macro GEN_KVM name
> -.endm
> + bne \handler
> #endif
> +.endm
>
> /*
> * This is the BOOK3S interrupt entry code macro.
> @@ -409,7 +379,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
> DEFINE_FIXED_SYMBOL(\name\()_common_real)
> \name\()_common_real:
> .if IKVM_REAL
> - KVMTEST \name
> + KVMTEST \name kvm_interrupt
> .endif
>
> ld r10,PACAKMSR(r13) /* get MSR value for kernel */
> @@ -432,7 +402,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real)
> DEFINE_FIXED_SYMBOL(\name\()_common_virt)
> \name\()_common_virt:
> .if IKVM_VIRT
> - KVMTEST \name
> + KVMTEST \name kvm_interrupt
> 1:
> .endif
> .endif /* IVIRT */
> @@ -446,7 +416,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_virt)
> DEFINE_FIXED_SYMBOL(\name\()_common_real)
> \name\()_common_real:
> .if IKVM_REAL
> - KVMTEST \name
> + KVMTEST \name kvm_interrupt
> .endif
> .endm
>
> @@ -967,8 +937,6 @@ EXC_COMMON_BEGIN(system_reset_common)
> EXCEPTION_RESTORE_REGS
> RFI_TO_USER_OR_KERNEL
>
> - GEN_KVM system_reset
> -
>
> /**
> * Interrupt 0x200 - Machine Check Interrupt (MCE).
> @@ -1132,7 +1100,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
> /*
> * Check if we are coming from guest. If yes, then run the normal
> * exception handler which will take the
> - * machine_check_kvm->kvmppc_interrupt branch to deliver the MC event
> + * machine_check_kvm->kvm_interrupt branch to deliver the MC event
> * to guest.
> */
> lbz r11,HSTATE_IN_GUEST(r13)
> @@ -1203,8 +1171,6 @@ EXC_COMMON_BEGIN(machine_check_common)
> bl machine_check_exception
> b interrupt_return
>
> - GEN_KVM machine_check
> -
>
> #ifdef CONFIG_PPC_P7_NAP
> /*
> @@ -1339,8 +1305,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
> REST_NVGPRS(r1)
> b interrupt_return
>
> - GEN_KVM data_access
> -
>
> /**
> * Interrupt 0x380 - Data Segment Interrupt (DSLB).
> @@ -1390,8 +1354,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
> bl do_bad_slb_fault
> b interrupt_return
>
> - GEN_KVM data_access_slb
> -
>
> /**
> * Interrupt 0x400 - Instruction Storage Interrupt (ISI).
> @@ -1428,8 +1390,6 @@ MMU_FTR_SECTION_ELSE
> ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
> b interrupt_return
>
> - GEN_KVM instruction_access
> -
>
> /**
> * Interrupt 0x480 - Instruction Segment Interrupt (ISLB).
> @@ -1474,8 +1434,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
> bl do_bad_slb_fault
> b interrupt_return
>
> - GEN_KVM instruction_access_slb
> -
>
> /**
> * Interrupt 0x500 - External Interrupt.
> @@ -1521,8 +1479,6 @@ EXC_COMMON_BEGIN(hardware_interrupt_common)
> bl do_IRQ
> b interrupt_return
>
> - GEN_KVM hardware_interrupt
> -
>
> /**
> * Interrupt 0x600 - Alignment Interrupt
> @@ -1550,8 +1506,6 @@ EXC_COMMON_BEGIN(alignment_common)
> REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> b interrupt_return
>
> - GEN_KVM alignment
> -
>
> /**
> * Interrupt 0x700 - Program Interrupt (program check).
> @@ -1659,8 +1613,6 @@ EXC_COMMON_BEGIN(program_check_common)
> REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> b interrupt_return
>
> - GEN_KVM program_check
> -
>
> /*
> * Interrupt 0x800 - Floating-Point Unavailable Interrupt.
> @@ -1710,8 +1662,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
> b interrupt_return
> #endif
>
> - GEN_KVM fp_unavailable
> -
>
> /**
> * Interrupt 0x900 - Decrementer Interrupt.
> @@ -1751,8 +1701,6 @@ EXC_COMMON_BEGIN(decrementer_common)
> bl timer_interrupt
> b interrupt_return
>
> - GEN_KVM decrementer
> -
>
> /**
> * Interrupt 0x980 - Hypervisor Decrementer Interrupt.
> @@ -1798,8 +1746,6 @@ EXC_COMMON_BEGIN(hdecrementer_common)
> ld r13,PACA_EXGEN+EX_R13(r13)
> HRFI_TO_KERNEL
>
> - GEN_KVM hdecrementer
> -
>
> /**
> * Interrupt 0xa00 - Directed Privileged Doorbell Interrupt.
> @@ -1840,8 +1786,6 @@ EXC_COMMON_BEGIN(doorbell_super_common)
> #endif
> b interrupt_return
>
> - GEN_KVM doorbell_super
> -
>
> EXC_REAL_NONE(0xb00, 0x100)
> EXC_VIRT_NONE(0x4b00, 0x100)
> @@ -1891,7 +1835,7 @@ INT_DEFINE_END(system_call)
> GET_PACA(r13)
> std r10,PACA_EXGEN+EX_R10(r13)
> INTERRUPT_TO_KERNEL
> - KVMTEST system_call /* uses r10, branch to system_call_kvm */
> + KVMTEST system_call kvm_hcall /* uses r10, branch to kvm_hcall */
> mfctr r9
> #else
> mr r9,r13
> @@ -1947,7 +1891,7 @@ EXC_VIRT_BEGIN(system_call, 0x4c00, 0x100)
> EXC_VIRT_END(system_call, 0x4c00, 0x100)
>
> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> -TRAMP_REAL_BEGIN(system_call_kvm)
> +TRAMP_REAL_BEGIN(kvm_hcall)
> mfctr r10
> SET_SCRATCH0(r10) /* Save r13 in SCRATCH0 */
> #ifdef CONFIG_RELOCATABLE
> @@ -1987,8 +1931,6 @@ EXC_COMMON_BEGIN(single_step_common)
> bl single_step_exception
> b interrupt_return
>
> - GEN_KVM single_step
> -
>
> /**
> * Interrupt 0xe00 - Hypervisor Data Storage Interrupt (HDSI).
> @@ -2027,8 +1969,6 @@ MMU_FTR_SECTION_ELSE
> ALT_MMU_FTR_SECTION_END_IFSET(MMU_FTR_TYPE_RADIX)
> b interrupt_return
>
> - GEN_KVM h_data_storage
> -
>
> /**
> * Interrupt 0xe20 - Hypervisor Instruction Storage Interrupt (HISI).
> @@ -2054,8 +1994,6 @@ EXC_COMMON_BEGIN(h_instr_storage_common)
> bl unknown_exception
> b interrupt_return
>
> - GEN_KVM h_instr_storage
> -
>
> /**
> * Interrupt 0xe40 - Hypervisor Emulation Assistance Interrupt.
> @@ -2080,8 +2018,6 @@ EXC_COMMON_BEGIN(emulation_assist_common)
> REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> b interrupt_return
>
> - GEN_KVM emulation_assist
> -
>
> /**
> * Interrupt 0xe60 - Hypervisor Maintenance Interrupt (HMI).
> @@ -2153,8 +2089,6 @@ EXC_COMMON_BEGIN(hmi_exception_early_common)
> EXCEPTION_RESTORE_REGS hsrr=1
> GEN_INT_ENTRY hmi_exception, virt=0
>
> - GEN_KVM hmi_exception_early
> -
> EXC_COMMON_BEGIN(hmi_exception_common)
> GEN_COMMON hmi_exception
> FINISH_NAP
> @@ -2162,8 +2096,6 @@ EXC_COMMON_BEGIN(hmi_exception_common)
> bl handle_hmi_exception
> b interrupt_return
>
> - GEN_KVM hmi_exception
> -
>
> /**
> * Interrupt 0xe80 - Directed Hypervisor Doorbell Interrupt.
> @@ -2195,8 +2127,6 @@ EXC_COMMON_BEGIN(h_doorbell_common)
> #endif
> b interrupt_return
>
> - GEN_KVM h_doorbell
> -
>
> /**
> * Interrupt 0xea0 - Hypervisor Virtualization Interrupt.
> @@ -2224,8 +2154,6 @@ EXC_COMMON_BEGIN(h_virt_irq_common)
> bl do_IRQ
> b interrupt_return
>
> - GEN_KVM h_virt_irq
> -
>
> EXC_REAL_NONE(0xec0, 0x20)
> EXC_VIRT_NONE(0x4ec0, 0x20)
> @@ -2270,8 +2198,6 @@ EXC_COMMON_BEGIN(performance_monitor_common)
> bl performance_monitor_exception
> b interrupt_return
>
> - GEN_KVM performance_monitor
> -
>
> /**
> * Interrupt 0xf20 - Vector Unavailable Interrupt.
> @@ -2321,8 +2247,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> bl altivec_unavailable_exception
> b interrupt_return
>
> - GEN_KVM altivec_unavailable
> -
>
> /**
> * Interrupt 0xf40 - VSX Unavailable Interrupt.
> @@ -2371,8 +2295,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
> bl vsx_unavailable_exception
> b interrupt_return
>
> - GEN_KVM vsx_unavailable
> -
>
> /**
> * Interrupt 0xf60 - Facility Unavailable Interrupt.
> @@ -2401,8 +2323,6 @@ EXC_COMMON_BEGIN(facility_unavailable_common)
> REST_NVGPRS(r1) /* instruction emulation may change GPRs */
> b interrupt_return
>
> - GEN_KVM facility_unavailable
> -
>
> /**
> * Interrupt 0xf60 - Hypervisor Facility Unavailable Interrupt.
> @@ -2431,8 +2351,6 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common)
> REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */
> b interrupt_return
>
> - GEN_KVM h_facility_unavailable
> -
>
> EXC_REAL_NONE(0xfa0, 0x20)
> EXC_VIRT_NONE(0x4fa0, 0x20)
> @@ -2462,8 +2380,6 @@ EXC_COMMON_BEGIN(cbe_system_error_common)
> bl cbe_system_error_exception
> b interrupt_return
>
> - GEN_KVM cbe_system_error
> -
> #else /* CONFIG_CBE_RAS */
> EXC_REAL_NONE(0x1200, 0x100)
> EXC_VIRT_NONE(0x5200, 0x100)
> @@ -2495,8 +2411,6 @@ EXC_COMMON_BEGIN(instruction_breakpoint_common)
> bl instruction_breakpoint_exception
> b interrupt_return
>
> - GEN_KVM instruction_breakpoint
> -
>
> EXC_REAL_NONE(0x1400, 0x100)
> EXC_VIRT_NONE(0x5400, 0x100)
> @@ -2617,8 +2531,6 @@ EXC_COMMON_BEGIN(denorm_exception_common)
> bl unknown_exception
> b interrupt_return
>
> - GEN_KVM denorm_exception
> -
>
> #ifdef CONFIG_CBE_RAS
> INT_DEFINE_BEGIN(cbe_maintenance)
> @@ -2636,8 +2548,6 @@ EXC_COMMON_BEGIN(cbe_maintenance_common)
> bl cbe_maintenance_exception
> b interrupt_return
>
> - GEN_KVM cbe_maintenance
> -
> #else /* CONFIG_CBE_RAS */
> EXC_REAL_NONE(0x1600, 0x100)
> EXC_VIRT_NONE(0x5600, 0x100)
> @@ -2668,8 +2578,6 @@ EXC_COMMON_BEGIN(altivec_assist_common)
> #endif
> b interrupt_return
>
> - GEN_KVM altivec_assist
> -
>
> #ifdef CONFIG_CBE_RAS
> INT_DEFINE_BEGIN(cbe_thermal)
> @@ -2687,8 +2595,6 @@ EXC_COMMON_BEGIN(cbe_thermal_common)
> bl cbe_thermal_exception
> b interrupt_return
>
> - GEN_KVM cbe_thermal
> -
> #else /* CONFIG_CBE_RAS */
> EXC_REAL_NONE(0x1800, 0x100)
> EXC_VIRT_NONE(0x5800, 0x100)
> @@ -2941,6 +2847,15 @@ TRAMP_REAL_BEGIN(rfscv_flush_fallback)
>
> USE_TEXT_SECTION()
>
> +#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> +kvm_interrupt:
> + /*
> + * The conditional branch in KVMTEST can't reach all the way,
> + * make a stub.
> + */
> + b kvmppc_interrupt
> +#endif
> +
> _GLOBAL(do_uaccess_flush)
> UACCESS_FLUSH_FIXUP_SECTION
> nop
> diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
> index f527e16707db..2c9d106145e8 100644
> --- a/arch/powerpc/kvm/book3s_64_entry.S
> +++ b/arch/powerpc/kvm/book3s_64_entry.S
> @@ -44,15 +44,61 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> sldi r12,r10,32
> ori r12,r12,0xc00
> ld r10,PACA_EXGEN+EX_R10(r13)
> + b do_kvm_interrupt
>
> +/*
> + * KVM interrupt entry occurs after GEN_INT_ENTRY runs, and follows that
> + * call convention:
> + *
> + * guest R9-R13, CTR, CFAR, PPR saved in PACA EX_xxx save area
> + * guest (H)DAR, (H)DSISR are also in the save area for relevant interrupts
> + * guest R13 also saved in SCRATCH0
> + * R13 = PACA
> + * R11 = (H)SRR0
> + * R12 = (H)SRR1
> + * R9 = guest CR
> + * PPR is set to medium
> + *
> + * With the addition for KVM:
> + * R10 = trap vector
> + */
> .global kvmppc_interrupt
> .balign IFETCH_ALIGN_BYTES
> kvmppc_interrupt:
> + li r11,PACA_EXGEN
> + cmpdi r10,0x200
> + bgt+ 1f
> + li r11,PACA_EXMC
> + beq 1f
> + li r11,PACA_EXNMI
> +1: add r11,r11,r13
> +
> +BEGIN_FTR_SECTION
> + ld r12,EX_CFAR(r11)
> + std r12,HSTATE_CFAR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
> + ld r12,EX_CTR(r11)
> + mtctr r12
> +BEGIN_FTR_SECTION
> + ld r12,EX_PPR(r11)
> + std r12,HSTATE_PPR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> + ld r12,EX_R12(r11)
> + std r12,HSTATE_SCRATCH0(r13)
> + sldi r12,r9,32
> + or r12,r12,r10
> + ld r9,EX_R9(r11)
> + ld r10,EX_R10(r11)
> + ld r11,EX_R11(r11)
> +
> +do_kvm_interrupt:
> /*
> - * Register contents:
> + * Hcalls and other interrupts come here after normalising register
> + * contents and save locations:
> + *
> * R12 = (guest CR << 32) | interrupt vector
> * R13 = PACA
> - * guest R12 saved in shadow VCPU SCRATCH0
> + * guest R12 saved in shadow HSTATE_SCRATCH0
> * guest R13 saved in SPRN_SCRATCH0
> */
> std r9,HSTATE_SCRATCH2(r13)
^ permalink raw reply
* Re: [PATCH v1 4/7] KVM: PPC: Book3S 64: Move hcall early register setup to KVM
From: Fabiano Rosas @ 2021-04-16 18:28 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412075103.1533302-5-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> System calls / hcalls have a different calling convention than
> other interrupts, so there is code in the KVMTEST to massage these
> into the same form as other interrupt handlers.
>
> Move this work into the KVM hcall handler. This means teaching KVM
> a little more about the low level interrupt handler setup, PACA save
> areas, etc., although that's not obviously worse than the current
> approach of coming up with an entirely different interrupt register
> / save convention.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/include/asm/exception-64s.h | 13 ++++++++
> arch/powerpc/kernel/exceptions-64s.S | 42 +-----------------------
> arch/powerpc/kvm/book3s_64_entry.S | 30 +++++++++++++++++
> 3 files changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> index c1a8aac01cf9..bb6f78fcf981 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -35,6 +35,19 @@
> /* PACA save area size in u64 units (exgen, exmc, etc) */
> #define EX_SIZE 10
>
> +/* PACA save area offsets */
> +#define EX_R9 0
> +#define EX_R10 8
> +#define EX_R11 16
> +#define EX_R12 24
> +#define EX_R13 32
> +#define EX_DAR 40
> +#define EX_DSISR 48
> +#define EX_CCR 52
> +#define EX_CFAR 56
> +#define EX_PPR 64
> +#define EX_CTR 72
> +
> /*
> * maximum recursive depth of MCE exceptions
> */
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 9467fd1038f9..1bfd0d7af09e 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -21,22 +21,6 @@
> #include <asm/feature-fixups.h>
> #include <asm/kup.h>
>
> -/* PACA save area offsets (exgen, exmc, etc) */
> -#define EX_R9 0
> -#define EX_R10 8
> -#define EX_R11 16
> -#define EX_R12 24
> -#define EX_R13 32
> -#define EX_DAR 40
> -#define EX_DSISR 48
> -#define EX_CCR 52
> -#define EX_CFAR 56
> -#define EX_PPR 64
> -#define EX_CTR 72
> -.if EX_SIZE != 10
> - .error "EX_SIZE is wrong"
> -.endif
> -
> /*
> * Following are fixed section helper macros.
> *
> @@ -1964,29 +1948,8 @@ EXC_VIRT_END(system_call, 0x4c00, 0x100)
>
> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> TRAMP_REAL_BEGIN(system_call_kvm)
> - /*
> - * This is a hcall, so register convention is as above, with these
> - * differences:
> - * r13 = PACA
> - * ctr = orig r13
> - * orig r10 saved in PACA
> - */
> - /*
> - * Save the PPR (on systems that support it) before changing to
> - * HMT_MEDIUM. That allows the KVM code to save that value into the
> - * guest state (it is the guest's PPR value).
> - */
> -BEGIN_FTR_SECTION
> - mfspr r10,SPRN_PPR
> - std r10,HSTATE_PPR(r13)
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> - HMT_MEDIUM
> mfctr r10
> - SET_SCRATCH0(r10)
> - mfcr r10
> - std r12,HSTATE_SCRATCH0(r13)
> - sldi r12,r10,32
> - ori r12,r12,0xc00
> + SET_SCRATCH0(r10) /* Save r13 in SCRATCH0 */
> #ifdef CONFIG_RELOCATABLE
> /*
> * Requires __LOAD_FAR_HANDLER beause kvmppc_hcall lives
> @@ -1994,15 +1957,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> */
> __LOAD_FAR_HANDLER(r10, kvmppc_hcall)
> mtctr r10
> - ld r10,PACA_EXGEN+EX_R10(r13)
> bctr
> #else
> - ld r10,PACA_EXGEN+EX_R10(r13)
> b kvmppc_hcall
> #endif
> #endif
>
> -
> /**
> * Interrupt 0xd00 - Trace Interrupt.
> * This is a synchronous interrupt in response to instruction step or
> diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
> index c21fa64059ef..f527e16707db 100644
> --- a/arch/powerpc/kvm/book3s_64_entry.S
> +++ b/arch/powerpc/kvm/book3s_64_entry.S
> @@ -14,6 +14,36 @@
> .global kvmppc_hcall
> .balign IFETCH_ALIGN_BYTES
> kvmppc_hcall:
> + /*
> + * This is a hcall, so register convention is as
> + * Documentation/powerpc/papr_hcalls.rst, with these additions:
> + * R13 = PACA
> + * guest R13 saved in SPRN_SCRATCH0
> + * R10 = free
> + * guest r10 saved in PACA_EXGEN
> + *
> + * This may also be a syscall from PR-KVM userspace that is to be
> + * reflected to the PR guest kernel, so registers may be set up for
> + * a system call rather than hcall. We don't currently clobber
> + * anything here, but the 0xc00 handler has already clobbered CTR
> + * and CR0, so PR-KVM can not support a guest kernel that preserves
> + * those registers across its system calls.
> + */
> + /*
> + * Save the PPR (on systems that support it) before changing to
> + * HMT_MEDIUM. That allows the KVM code to save that value into the
> + * guest state (it is the guest's PPR value).
> + */
> +BEGIN_FTR_SECTION
> + mfspr r10,SPRN_PPR
> + std r10,HSTATE_PPR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> + HMT_MEDIUM
> + mfcr r10
> + std r12,HSTATE_SCRATCH0(r13)
> + sldi r12,r10,32
> + ori r12,r12,0xc00
> + ld r10,PACA_EXGEN+EX_R10(r13)
>
> .global kvmppc_interrupt
> .balign IFETCH_ALIGN_BYTES
^ permalink raw reply
* Re: [PATCH v1 12/12] KVM: PPC: Book3S HV: Ensure MSR[HV] is always clear in guest MSR
From: Fabiano Rosas @ 2021-04-16 18:34 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412014845.1517916-13-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Rather than clear the HV bit from the MSR at guest entry, make it clear
> that the hypervisor does not allow the guest to set the bit.
>
> The HV clear is kept in guest entry for now, but a future patch will
> warn if it is set.
>
> Acked-by: Paul Mackerras <paulus@ozlabs.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_builtin.c | 4 ++--
> arch/powerpc/kvm/book3s_hv_nested.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
> index 41cb03d0bde4..7a0e33a9c980 100644
> --- a/arch/powerpc/kvm/book3s_hv_builtin.c
> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
> @@ -662,8 +662,8 @@ static void kvmppc_end_cede(struct kvm_vcpu *vcpu)
>
> void kvmppc_set_msr_hv(struct kvm_vcpu *vcpu, u64 msr)
> {
> - /* Guest must always run with ME enabled. */
> - msr = msr | MSR_ME;
> + /* Guest must always run with ME enabled, HV disabled. */
> + msr = (msr | MSR_ME) & ~MSR_HV;
>
> /*
> * Check for illegal transactional state bit combination
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index fb03085c902b..60724f674421 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -344,8 +344,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> vcpu->arch.nested_vcpu_id = l2_hv.vcpu_token;
> vcpu->arch.regs = l2_regs;
>
> - /* Guest must always run with ME enabled. */
> - vcpu->arch.shregs.msr = vcpu->arch.regs.msr | MSR_ME;
> + /* Guest must always run with ME enabled, HV disabled. */
> + vcpu->arch.shregs.msr = (vcpu->arch.regs.msr | MSR_ME) & ~MSR_HV;
>
> sanitise_hv_regs(vcpu, &l2_hv);
> restore_hv_regs(vcpu, &l2_hv);
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Add shutdown() to vio_driver and vio_bus
From: Tyrel Datwyler @ 2021-04-16 18:46 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210402001325.939668-1-tyreld@linux.ibm.com>
On 4/1/21 5:13 PM, Tyrel Datwyler wrote:
> Currently, neither the vio_bus or vio_driver structures provide support
> for a shutdown() routine.
>
> Add support for shutdown() by allowing drivers to provide a
> implementation via function pointer in their vio_driver struct and
> provide a proper implementation in the driver template for the vio_bus
> that calls a vio drivers shutdown() if defined.
>
> In the case that no shutdown() is defined by a vio driver and a kexec is
> in progress we implement a big hammer that calls remove() to ensure no
> further DMA for the devices is possible.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
Ping... any comments, problems with this approach?
-Tyrel
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
From: Tyrel Datwyler @ 2021-04-16 20:21 UTC (permalink / raw)
To: Daniel Axtens, mpe; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <878s5ig0p7.fsf@linkitivity.dja.id.au>
On 4/16/21 12:15 AM, Daniel Axtens wrote:
> Hi Tyrel,
>
>> The pci_bus->bridge reference may no longer be valid after
>> pci_bus_remove() resulting in passing a bad value to device_unregister()
>> for the associated bridge device.
>>
>> Store the host_bridge reference in a separate variable prior to
>> pci_bus_remove().
>>
> The patch certainly seems to do what you say. I'm not really up on the
> innards of PCI, so I'm struggling to figure out by what code path
> pci_bus_remove() might invalidate pci_bus->bridge? A quick look at
> pci_remove_bus was not very illuminating but I didn't chase down every
> call it made.
remove_phb_dynamic()
|--> pci_remove_bus(bus)
|--> device_unregister(&bus->dev)
|--> put_device(dev)
|--> device_release(kobj)
|--> dev->class->dev_release(dev) == release_pci_bus(dev)
|--> kfree(bus)
We have the above call chain that takes place in the when put_device() triggers
the kobject ref count to go zero. The kobject_release function in this case is
device_release() which in turn calls dev->class->dev_release(dev). For a pci_bus
the class is appropriately pcibus_class whose dev_release() callback points to
release_pci_bus(). This in turn calls kfree() on the bus. Which means we can no
longer safely dereference any fields of the pci_bus struct.
-Tyrel
>
> Kind regards,
> Daniel
>
>> Fixes: 7340056567e3 ("powerpc/pci: Reorder pci bus/bridge unregistration during PHB removal")
>> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/pci_dlpar.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> index f9ae17e8a0f4..a8f9140a24fa 100644
>> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> @@ -50,6 +50,7 @@ EXPORT_SYMBOL_GPL(init_phb_dynamic);
>> int remove_phb_dynamic(struct pci_controller *phb)
>> {
>> struct pci_bus *b = phb->bus;
>> + struct pci_host_bridge *host_bridge = to_pci_host_bridge(b->bridge);
>> struct resource *res;
>> int rc, i;
>>
>> @@ -76,7 +77,8 @@ int remove_phb_dynamic(struct pci_controller *phb)
>> /* Remove the PCI bus and unregister the bridge device from sysfs */
>> phb->bus = NULL;
>> pci_remove_bus(b);
>> - device_unregister(b->bridge);
>> + host_bridge->bus = NULL;
>> + device_unregister(&host_bridge->dev);
>>
>> /* Now release the IO resource */
>> if (res->flags & IORESOURCE_IO)
>> --
>> 2.27.0
^ permalink raw reply
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Leonardo Bras @ 2021-04-16 20:57 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <CAL_Jsq+WwAeziGN4EfPAWfA0fieAjfcxfi29=StOx0GeKjAe_g@mail.gmail.com>
Hello Rob, thanks for this feedback!
On Thu, 2021-04-15 at 13:59 -0500, Rob Herring wrote:
> +PPC and PCI lists
>
> On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> >
> > Many other resource flag parsers already add this flag when the input
> > has bits 24 & 25 set, so update this one to do the same.
>
> Many others? Looks like sparc and powerpc to me.
>
s390 also does that, but it look like it comes from a device-tree.
> Those would be the
> ones I worry about breaking. Sparc doesn't use of/address.c so it's
> fine. Powerpc version of the flags code was only fixed in 2019, so I
> don't think powerpc will care either.
In powerpc I reach this function with this stack, while configuring a
virtio-net device for a qemu/KVM pseries guest:
pci_process_bridge_OF_ranges+0xac/0x2d4
pSeries_discover_phbs+0xc4/0x158
discover_phbs+0x40/0x60
do_one_initcall+0x60/0x2d0
kernel_init_freeable+0x308/0x3a8
kernel_init+0x2c/0x168
ret_from_kernel_thread+0x5c/0x70
For this, both MMIO32 and MMIO64 resources will have flags 0x200.
>
> I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
> the flags. AFAICT, that's not set anywhere outside of arch code. So
> never for riscv, arm and arm64 at least. That leads me to
> pci_std_update_resource() which is where the PCI code sets BARs and
> just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
> IORESOURCE_* flags. So it seems like 64-bit is still not handled and
> neither is prefetch.
>
I am not sure if you mean here:
a) it's ok to add IORESOURCE_MEM_64 here, because it does not affect
anything else, or
b) it should be using PCI_BASE_ADDRESS_MEM_TYPE_64
(or IORESOURCE_MEM_64 | PCI_BASE_ADDRESS_MEM_TYPE_64) instead, since
it's how it's added in powerpc/sparc, and else there is no point.
Again, thanks for helping!
Best regards,
Leonardo Bras
^ permalink raw reply
* [PATCH 0/2] pseries: UNISOLATE DRCs to signal device removal error
From: Daniel Henrique Barboza @ 2021-04-16 21:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
At this moment, PAPR [1] does not have a way to report errors during a device
removal operation. This puts a strain in the hypervisor, which needs extra
mechanisms to try to fallback and recover from an error that might have
happened during the removal. The QEMU community has dealt with it during these
years by either trying to preempt the error before sending the HP event or, in
case of a guest side failure, reboot the guest to complete the removal process.
This started to change with QEMU commit fe1831eff8a4 ("spapr_drc.c: use DRC
reconfiguration to cleanup DIMM unplug state"), where a way to fallback from a
memory removal error was introduced. In this case, when QEMU detects that the
kernel is reconfiguring LMBs DRCs that were marked as pending removal, the
entire process is reverted from the QEMU side as well. Around the same time,
other discussions in the QEMU mailing discussed an alternative for other device
as well.
In [2] the idea of using RTAS set-indicator for this role was first introduced.
The RTAS set-indicator call, when attempting to UNISOLATE a DRC that is already
UNISOLATED or CONFIGURED, returns RTAS_OK and does nothing else for both QEMU
and phyp. This gives us an opportunity to use this behavior to signal the
hypervisor layer when a device removal happens, allowing it to do a proper
error handling knowing for sure that the removal failed in the kernel. Using
set-indicator to report HP errors isn't strange to PAPR, as per R1-13.5.3.4-4.
of table 13.7 of [1]:
"For all DR options: If this is a DR operation that involves the user insert-
ing a DR entity, then if the firmware can determine that the inserted entity
would cause a system disturbance, then the set-indicator RTAS call must not
unisolate the entity and must return an error status which is unique to the
particular error."
PAPR does not make any restrictions or considerations about setting an already
Unisolated/Configured DRC to 'unisolate', meaning we have a chance to use it
for this purpose - signal an OS side error when attempting to remove a DR
entity. To validate the design, this is being implemented only for CPUs.
QEMU will use this mechanism to rollback the device removal (hotunplug) state,
allowing for a better error handling mechanism. A implementation of how QEMU
can do it is in [3]. When using a kernel with this series applied, together
with this QEMU build, this is what happens in a common CPU removal/hotunplug
error scenario (trying to remove the last online CPU):
( QEMU command line: qemu-system-ppc64 -machine pseries,accel=kvm,usb=off
-smp 1,maxcpus=2,threads=1,cores=2,sockets=1 ... )
[root@localhost ~]# QEMU 5.2.92 monitor - type 'help' for more information
(qemu) device_add host-spapr-cpu-core,core-id=1,id=core1
(qemu)
[root@localhost ~]# echo 0 > /sys/devices/system/cpu/cpu0/online
[ 77.548442][ T13] IRQ 19: no longer affine to CPU0
[ 77.548452][ T13] IRQ 20: no longer affine to CPU0
[ 77.548458][ T13] IRQ 256: no longer affine to CPU0
[ 77.548465][ T13] IRQ 258: no longer affine to CPU0
[ 77.548472][ T13] IRQ 259: no longer affine to CPU0
[ 77.548479][ T13] IRQ 260: no longer affine to CPU0
[ 77.548485][ T13] IRQ 261: no longer affine to CPU0
[ 77.548590][ T0] cpu 0 (hwid 0) Ready to die...
[root@localhost ~]# (qemu)
(qemu) device_del core1
(qemu) [ 83.214073][ T100] pseries-hotplug-cpu: Failed to offline CPU PowerPC,POWER9, rc: -16
qemu-system-ppc64: Device hotunplug rejected by the guest for device core1
(qemu)
As soon as the CPU removal fails in dlpar_cpu(), QEMU becames aware of
it and is able to do error recovery.
If this solution is well received, I'll push for an architecture change
request internally at IBM to make this mechanism PAPR official.
[1] https://openpowerfoundation.org/wp-content/uploads/2020/07/LoPAR-20200611.pdf
[2] https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg06395.html
[3] https://github.com/danielhb/qemu/tree/unisolate_drc_callback_v1
Daniel Henrique Barboza (2):
dlpar.c: introduce dlpar_unisolate_drc()
hotplug-cpu.c: set UNISOLATE on dlpar_cpu_remove() failure
arch/powerpc/platforms/pseries/dlpar.c | 14 ++++++++++++++
arch/powerpc/platforms/pseries/hotplug-cpu.c | 9 ++++++++-
arch/powerpc/platforms/pseries/pseries.h | 1 +
3 files changed, 23 insertions(+), 1 deletion(-)
--
2.30.2
^ permalink raw reply
* [PATCH 1/2] dlpar.c: introduce dlpar_unisolate_drc()
From: Daniel Henrique Barboza @ 2021-04-16 21:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
In-Reply-To: <20210416210216.380291-1-danielhb413@gmail.com>
Next patch will execute a set-indicator call in hotplug-cpu.c.
Create a dlpar_unisolate_drc() helper to avoid spreading more
rtas_set_indicator() calls outside of dlpar.c.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 14 ++++++++++++++
arch/powerpc/platforms/pseries/pseries.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 233503fcf8f0..3ac70790ec7a 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -329,6 +329,20 @@ int dlpar_release_drc(u32 drc_index)
return 0;
}
+int dlpar_unisolate_drc(u32 drc_index)
+{
+ int dr_status, rc;
+
+ rc = rtas_call(rtas_token("get-sensor-state"), 2, 2, &dr_status,
+ DR_ENTITY_SENSE, drc_index);
+ if (rc || dr_status != DR_ENTITY_PRESENT)
+ return -1;
+
+ rtas_set_indicator(ISOLATION_STATE, drc_index, UNISOLATE);
+
+ return 0;
+}
+
int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
{
int rc;
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 4fe48c04c6c2..4ea12037c920 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -55,6 +55,7 @@ extern int dlpar_attach_node(struct device_node *, struct device_node *);
extern int dlpar_detach_node(struct device_node *);
extern int dlpar_acquire_drc(u32 drc_index);
extern int dlpar_release_drc(u32 drc_index);
+extern int dlpar_unisolate_drc(u32 drc_index);
void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog);
int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_errlog);
--
2.30.2
^ permalink raw reply related
* [PATCH 2/2] hotplug-cpu.c: set UNISOLATE on dlpar_cpu_remove() failure
From: Daniel Henrique Barboza @ 2021-04-16 21:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
In-Reply-To: <20210416210216.380291-1-danielhb413@gmail.com>
The RTAS set-indicator call, when attempting to UNISOLATE a DRC that is
already UNISOLATED or CONFIGURED, returns RTAS_OK and does nothing else
for both QEMU and phyp. This gives us an opportunity to use this
behavior to signal the hypervisor layer when an error during device
removal happens, allowing it to do a proper error handling, while not
breaking QEMU/phyp implementations that don't have this support.
This patch introduces this idea by unisolating all CPU DRCs that failed
to be removed by dlpar_cpu_remove_by_index(), when handling the
PSERIES_HP_ELOG_ID_DRC_INDEX event. This is being done for this event
only because its the only CPU removal event QEMU uses, and there's no
need at this moment to add this mechanism for phyp only code.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 12cbffd3c2e3..ed66895c2f51 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -802,8 +802,15 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
case PSERIES_HP_ELOG_ACTION_REMOVE:
if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
rc = dlpar_cpu_remove_by_count(count);
- else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
+ else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX) {
rc = dlpar_cpu_remove_by_index(drc_index);
+ /* Setting the isolation state of an UNISOLATED/CONFIGURED
+ * device to UNISOLATE is a no-op, but the hypervison can
+ * use it as a hint that the cpu removal failed.
+ */
+ if (rc)
+ dlpar_unisolate_drc(drc_index);
+ }
else
rc = -EINVAL;
break;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Leonardo Bras @ 2021-04-16 21:27 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <CAL_Jsq+WwAeziGN4EfPAWfA0fieAjfcxfi29=StOx0GeKjAe_g@mail.gmail.com>
Hello Rob, thanks for this feedback!
On Thu, 2021-04-15 at 13:59 -0500, Rob Herring wrote:
> +PPC and PCI lists
>
> On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> >
> > Many other resource flag parsers already add this flag when the input
> > has bits 24 & 25 set, so update this one to do the same.
>
> Many others? Looks like sparc and powerpc to me.
>
s390 also does that, but it look like it comes from a device-tree.
> Those would be the
> ones I worry about breaking. Sparc doesn't use of/address.c so it's
> fine. Powerpc version of the flags code was only fixed in 2019, so I
> don't think powerpc will care either.
In powerpc I reach this function with this stack, while configuring a
virtio-net device for a qemu/KVM pseries guest:
pci_process_bridge_OF_ranges+0xac/0x2d4
pSeries_discover_phbs+0xc4/0x158
discover_phbs+0x40/0x60
do_one_initcall+0x60/0x2d0
kernel_init_freeable+0x308/0x3a8
kernel_init+0x2c/0x168
ret_from_kernel_thread+0x5c/0x70
For this, both MMIO32 and MMIO64 resources will have flags 0x200.
>
> I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
> the flags. AFAICT, that's not set anywhere outside of arch code. So
> never for riscv, arm and arm64 at least. That leads me to
> pci_std_update_resource() which is where the PCI code sets BARs and
> just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
> IORESOURCE_* flags. So it seems like 64-bit is still not handled and
> neither is prefetch.
>
I am not sure if you mean here:
a) it's ok to add IORESOURCE_MEM_64 here, because it does not affect
anything else, or
b) it should be using PCI_BASE_ADDRESS_MEM_TYPE_64
(or IORESOURCE_MEM_64 | PCI_BASE_ADDRESS_MEM_TYPE_64) instead, since
it's how it's added in powerpc/sparc, and else there is no point.
Again, thanks for helping!
Best regards,
Leonardo Bras
^ permalink raw reply
* [PATCH 0/2] Change struct page layout for page_pool
From: Matthew Wilcox (Oracle) @ 2021-04-16 23:07 UTC (permalink / raw)
To: brouer
Cc: arnd, grygorii.strashko, netdev, ilias.apalodimas, linux-kernel,
Matthew Wilcox (Oracle), linux-mips, mhocko, linux-mm, mgorman,
mcroce, linux-snps-arc, linuxppc-dev, hch, linux-arm-kernel
The first patch here fixes two bugs on ppc32, and mips32. It fixes one
bug on arc and arm32 (in certain configurations). It probably makes
sense to get it in ASAP through the networking tree. I'd like to see
testing on those four architectures if possible?
The second patch enables new functionality. It is much less urgent.
I'd really like to see Mel & Michal's thoughts on it.
I have only compile-tested these patches.
Matthew Wilcox (Oracle) (2):
mm: Fix struct page layout on 32-bit systems
mm: Indicate pfmemalloc pages in compound_head
include/linux/mm.h | 12 +++++++-----
include/linux/mm_types.h | 9 ++++-----
include/net/page_pool.h | 12 +++++++++++-
net/core/page_pool.c | 12 +++++++-----
4 files changed, 29 insertions(+), 16 deletions(-)
--
2.30.2
^ permalink raw reply
* [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
From: Matthew Wilcox (Oracle) @ 2021-04-16 23:07 UTC (permalink / raw)
To: brouer
Cc: arnd, grygorii.strashko, netdev, ilias.apalodimas, linux-kernel,
Matthew Wilcox (Oracle), linux-mips, mhocko, linux-mm, mgorman,
mcroce, linux-snps-arc, linuxppc-dev, hch, linux-arm-kernel
In-Reply-To: <20210416230724.2519198-1-willy@infradead.org>
32-bit architectures which expect 8-byte alignment for 8-byte integers
and need 64-bit DMA addresses (arc, arm, mips, ppc) had their struct
page inadvertently expanded in 2019. When the dma_addr_t was added,
it forced the alignment of the union to 8 bytes, which inserted a 4 byte
gap between 'flags' and the union.
Fix this by storing the dma_addr_t in one or two adjacent unsigned longs.
This restores the alignment to that of an unsigned long, and also fixes a
potential problem where (on a big endian platform), the bit used to denote
PageTail could inadvertently get set, and a racing get_user_pages_fast()
could dereference a bogus compound_head().
Fixes: c25fff7171be ("mm: add dma_addr_t to struct page")
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/linux/mm_types.h | 4 ++--
include/net/page_pool.h | 12 +++++++++++-
net/core/page_pool.c | 12 +++++++-----
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 6613b26a8894..5aacc1c10a45 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -97,10 +97,10 @@ struct page {
};
struct { /* page_pool used by netstack */
/**
- * @dma_addr: might require a 64-bit value even on
+ * @dma_addr: might require a 64-bit value on
* 32-bit architectures.
*/
- dma_addr_t dma_addr;
+ unsigned long dma_addr[2];
};
struct { /* slab, slob and slub */
union {
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index b5b195305346..db7c7020746a 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -198,7 +198,17 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
{
- return page->dma_addr;
+ dma_addr_t ret = page->dma_addr[0];
+ if (sizeof(dma_addr_t) > sizeof(unsigned long))
+ ret |= (dma_addr_t)page->dma_addr[1] << 32;
+ return ret;
+}
+
+static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
+{
+ page->dma_addr[0] = addr;
+ if (sizeof(dma_addr_t) > sizeof(unsigned long))
+ page->dma_addr[1] = addr >> 32;
}
static inline bool is_page_pool_compiled_in(void)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index ad8b0707af04..f014fd8c19a6 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -174,8 +174,10 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool,
struct page *page,
unsigned int dma_sync_size)
{
+ dma_addr_t dma_addr = page_pool_get_dma_addr(page);
+
dma_sync_size = min(dma_sync_size, pool->p.max_len);
- dma_sync_single_range_for_device(pool->p.dev, page->dma_addr,
+ dma_sync_single_range_for_device(pool->p.dev, dma_addr,
pool->p.offset, dma_sync_size,
pool->p.dma_dir);
}
@@ -226,7 +228,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
put_page(page);
return NULL;
}
- page->dma_addr = dma;
+ page_pool_set_dma_addr(page, dma);
if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
@@ -294,13 +296,13 @@ void page_pool_release_page(struct page_pool *pool, struct page *page)
*/
goto skip_dma_unmap;
- dma = page->dma_addr;
+ dma = page_pool_get_dma_addr(page);
- /* When page is unmapped, it cannot be returned our pool */
+ /* When page is unmapped, it cannot be returned to our pool */
dma_unmap_page_attrs(pool->p.dev, dma,
PAGE_SIZE << pool->p.order, pool->p.dma_dir,
DMA_ATTR_SKIP_CPU_SYNC);
- page->dma_addr = 0;
+ page_pool_set_dma_addr(page, 0);
skip_dma_unmap:
/* This may be the last page returned, releasing the pool, so
* it is not safe to reference pool afterwards.
--
2.30.2
^ permalink raw reply related
* [PATCH 2/2] mm: Indicate pfmemalloc pages in compound_head
From: Matthew Wilcox (Oracle) @ 2021-04-16 23:07 UTC (permalink / raw)
To: brouer
Cc: arnd, grygorii.strashko, netdev, ilias.apalodimas, linux-kernel,
Matthew Wilcox (Oracle), linux-mips, mhocko, linux-mm, mgorman,
mcroce, linux-snps-arc, linuxppc-dev, hch, linux-arm-kernel
In-Reply-To: <20210416230724.2519198-1-willy@infradead.org>
The net page_pool wants to use a magic value to identify page pool pages.
The best place to put it is in the first word where it can be clearly a
non-pointer value. That means shifting dma_addr up to alias with ->index,
which means we need to find another way to indicate page_is_pfmemalloc().
Since page_pool doesn't want to set its magic value on pages which are
pfmemalloc, we can use bit 1 of compound_head to indicate that the page
came from the memory reserves.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/linux/mm.h | 12 +++++++-----
include/linux/mm_types.h | 7 +++----
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8ba434287387..44eab3f6d5ae 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1629,10 +1629,12 @@ struct address_space *page_mapping_file(struct page *page);
static inline bool page_is_pfmemalloc(const struct page *page)
{
/*
- * Page index cannot be this large so this must be
- * a pfmemalloc page.
+ * This is not a tail page; compound_head of a head page is unused
+ * at return from the page allocator, and will be overwritten
+ * by callers who do not care whether the page came from the
+ * reserves.
*/
- return page->index == -1UL;
+ return page->compound_head & 2;
}
/*
@@ -1641,12 +1643,12 @@ static inline bool page_is_pfmemalloc(const struct page *page)
*/
static inline void set_page_pfmemalloc(struct page *page)
{
- page->index = -1UL;
+ page->compound_head = 2;
}
static inline void clear_page_pfmemalloc(struct page *page)
{
- page->index = 0;
+ page->compound_head = 0;
}
/*
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5aacc1c10a45..39f7163dcace 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -96,10 +96,9 @@ struct page {
unsigned long private;
};
struct { /* page_pool used by netstack */
- /**
- * @dma_addr: might require a 64-bit value on
- * 32-bit architectures.
- */
+ unsigned long pp_magic;
+ unsigned long xmi;
+ unsigned long _pp_mapping_pad;
unsigned long dma_addr[2];
};
struct { /* slab, slob and slub */
--
2.30.2
^ permalink raw reply related
* Re: [PATCH net-next 0/2] net: gianfar: Drop GFAR_MQ_POLLING support
From: patchwork-bot+netdevbpf @ 2021-04-17 0:00 UTC (permalink / raw)
To: Claudiu Manoil
Cc: devicetree, vladimir.oltean, robh+dt, netdev, kuba, linuxppc-dev,
davem
In-Reply-To: <20210416171123.22969-1-claudiu.manoil@nxp.com>
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Fri, 16 Apr 2021 20:11:21 +0300 you wrote:
> Drop long time obsolete "per NAPI multi-queue" support in gianfar,
> and related (and undocumented) device tree properties.
>
> Claudiu Manoil (2):
> gianfar: Drop GFAR_MQ_POLLING support
> powerpc: dts: fsl: Drop obsolete fsl,rx-bit-map and fsl,tx-bit-map
> properties
>
> [...]
Here is the summary with links:
- [net-next,1/2] gianfar: Drop GFAR_MQ_POLLING support
https://git.kernel.org/netdev/net-next/c/8eda54c5e6c4
- [net-next,2/2] powerpc: dts: fsl: Drop obsolete fsl,rx-bit-map and fsl,tx-bit-map properties
https://git.kernel.org/netdev/net-next/c/221e8c126b78
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: Remove bpf_jit_enable=2 debugging mode
From: Jianlin Lv @ 2021-04-16 13:38 UTC (permalink / raw)
To: Daniel Borkmann
Cc: irogers, Song Liu, linux-doc, zlim.lnx, paul.walmsley,
Alexei Starovoitov, Andrii Nakryiko, paulus, sandipan, hpa,
sparclinux, illusionist.neo, Mahesh Bandewar, Will Deacon,
Nicolas Dichtel, linux-s390, iii, paulburton, corbet,
mchehab+huawei, Masahiro Yamada, x86, John Fastabend, linux,
linux-riscv, borntraeger, mingo, linux-arm-kernel,
catalin.marinas, naveen.n.rao, Jakub Kicinski, tklauser,
linux-mips, grantseltzer, xi.wang, aou, Kees Cook, gor,
luke.r.nels, LKML, hca, linuxppc-dev, KP Singh, horms, bp,
Alexander Viro, Yonghong Song, tglx, Dmitry Vyukov, tsbogend,
yoshfuji, Network Development, dsahern, udknight,
Martin KaFai Lau, bjorn, palmer, quentin, bpf, Jianlin Lv,
David S. Miller
In-Reply-To: <9c4a78d2-f73c-832a-e6e2-4b4daa729e07@iogearbox.net>
On Thu, Apr 15, 2021 at 10:38 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 4/15/21 11:32 AM, Jianlin Lv wrote:
> > For debugging JITs, dumping the JITed image to kernel log is discouraged,
> > "bpftool prog dump jited" is much better way to examine JITed dumps.
> > This patch get rid of the code related to bpf_jit_enable=2 mode and
> > update the proc handler of bpf_jit_enable, also added auxiliary
> > information to explain how to use bpf_jit_disasm tool after this change.
> >
> > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> [...]
> > diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> > index 0a7a2870f111..8d36b4658076 100644
> > --- a/arch/x86/net/bpf_jit_comp32.c
> > +++ b/arch/x86/net/bpf_jit_comp32.c
> > @@ -2566,9 +2566,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> > cond_resched();
> > }
> >
> > - if (bpf_jit_enable > 1)
> > - bpf_jit_dump(prog->len, proglen, pass + 1, image);
> > -
> > if (image) {
> > bpf_jit_binary_lock_ro(header);
> > prog->bpf_func = (void *)image;
> > diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> > index c8496c1142c9..990b1720c7a4 100644
> > --- a/net/core/sysctl_net_core.c
> > +++ b/net/core/sysctl_net_core.c
> > @@ -273,16 +273,8 @@ static int proc_dointvec_minmax_bpf_enable(struct ctl_table *table, int write,
> >
> > tmp.data = &jit_enable;
> > ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> > - if (write && !ret) {
> > - if (jit_enable < 2 ||
> > - (jit_enable == 2 && bpf_dump_raw_ok(current_cred()))) {
> > - *(int *)table->data = jit_enable;
> > - if (jit_enable == 2)
> > - pr_warn("bpf_jit_enable = 2 was set! NEVER use this in production, only for JIT debugging!\n");
> > - } else {
> > - ret = -EPERM;
> > - }
> > - }
> > + if (write && !ret)
> > + *(int *)table->data = jit_enable;
> > return ret;
> > }
> >
> > @@ -389,7 +381,7 @@ static struct ctl_table net_core_table[] = {
> > .extra2 = SYSCTL_ONE,
> > # else
> > .extra1 = SYSCTL_ZERO,
> > - .extra2 = &two,
> > + .extra2 = SYSCTL_ONE,
> > # endif
> > },
> > # ifdef CONFIG_HAVE_EBPF_JIT
> > diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
> > index c8ae95804728..efa4b17ae016 100644
> > --- a/tools/bpf/bpf_jit_disasm.c
> > +++ b/tools/bpf/bpf_jit_disasm.c
> > @@ -7,7 +7,7 @@
> > *
> > * To get the disassembly of the JIT code, do the following:
> > *
> > - * 1) `echo 2 > /proc/sys/net/core/bpf_jit_enable`
> > + * 1) Insert bpf_jit_dump() and recompile the kernel to output JITed image into log
>
> Hmm, if we remove bpf_jit_dump(), the next drive-by cleanup patch will be thrown
> at bpf@vger stating that bpf_jit_dump() has no in-tree users and should be removed.
> Maybe we should be removing bpf_jit_disasm.c along with it as well as bpf_jit_dump()
> itself ... I guess if it's ever needed in those rare occasions for JIT debugging we
> can resurrect it from old kernels just locally. But yeah, bpftool's jit dump should
> suffice for vast majority of use cases.
>
> There was a recent set for ppc32 jit which was merged into ppc tree which will create
> a merge conflict with this one [0]. So we would need a rebase and take it maybe during
> merge win once the ppc32 landed..
>
> [0] https://lore.kernel.org/bpf/cover.1616430991.git.christophe.leroy@csgroup.eu/
>
> > * 2) Load a BPF filter (e.g. `tcpdump -p -n -s 0 -i eth1 host 192.168.20.0/24`)
> > * 3) Run e.g. `bpf_jit_disasm -o` to read out the last JIT code
> > *
> > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > index 40a88df275f9..98c7eec2923f 100644
> > --- a/tools/bpf/bpftool/feature.c
> > +++ b/tools/bpf/bpftool/feature.c
> > @@ -203,9 +203,6 @@ static void probe_jit_enable(void)
> > case 1:
> > printf("JIT compiler is enabled\n");
> > break;
> > - case 2:
> > - printf("JIT compiler is enabled with debugging traces in kernel logs\n");
> > - break;
>
> This would still need to be there for older kernels ...
I will submit another version after ppc32 landed to remove
bpf_jit_disasm.c and restore bpftool/feature.c
Jianlin
>
> > case -1:
> > printf("Unable to retrieve JIT-compiler status\n");
> > break;
> >
>
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices
From: Rob Herring @ 2021-04-16 15:19 UTC (permalink / raw)
To: Michael Walle
Cc: Andrew Lunn, Paul Mackerras, Rafał Miłecki,
Nobuhiro Iwamatsu, moderated list:ARM/STM32 ARCHITECTURE,
Jerome Brunet, Neil Armstrong, Michal Simek, Jose Abreu,
NXP Linux Team, Mark Lee, Hauke Mehrtens, Sascha Hauer,
Lorenzo Bianconi, linux-omap, Greg Kroah-Hartman, linux-wireless,
linux-kernel@vger.kernel.org, Pengutronix Kernel Team,
Vladimir Oltean, Claudiu Beznea, Jérôme Pouiller,
Kunihiko Hayashi, Chris Snook, Frank Rowand, Gregory Clement,
Madalin Bucur, Martin Blumenstingl, Murali Karicheri,
Yisen Zhuang, Alexandre Torgue, Wingman Kwok, Sean Wang,
Maxime Ripard, Claudiu Manoil, open list:ARM/Amlogic Meson...,
Kalle Valo, Mirko Lindner, Fugang Duan, Bryan Whitehead,
QCA ath9k Development, Microchip Linux Driver Support,
Taras Chornyi, Maxime Coquelin, Kevin Hilman, Heiner Kallweit,
Andreas Larsson, Giuseppe Cavallaro, Fabio Estevam,
Stanislaw Gruszka, Florian Fainelli, linux-staging, Chen-Yu Tsai,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, linux-arm-kernel,
Grygorii Strashko, Byungho An, Radhey Shyam Pandey,
Vladimir Zapolskiy, John Crispin, Salil Mehta, Sergei Shtylyov,
linux-oxnas, Shawn Guo, David S . Miller, Helmut Schaa,
Thomas Petazzoni, open list:MEDIA DRIVERS FOR RENESAS - FCP,
Ryder Lee, Russell King, Vadym Kochan, Jakub Kicinski,
Vivien Didelot, Sunil Goutham, Sebastian Hesselbarth, devicetree,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Jernej Skrabec, netdev, Nicolas Ferre, Li Yang, Stephen Hemminger,
Vinod Koul, Joyce Ooi, linuxppc-dev, Felix Fietkau
In-Reply-To: <8157eba9317609294da80472622deb28@walle.cc>
On Fri, Apr 16, 2021 at 2:30 AM Michael Walle <michael@walle.cc> wrote:
>
> Am 2021-04-16 05:24, schrieb Benjamin Herrenschmidt:
> > On Mon, 2021-04-12 at 19:47 +0200, Michael Walle wrote:
> >>
> >> /**
> >> * of_get_phy_mode - Get phy mode for given device_node
> >> @@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np,
> >> const char *name, u8 *addr)
> >> static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
> >> {
> >> struct platform_device *pdev = of_find_device_by_node(np);
> >> + struct nvmem_cell *cell;
> >> + const void *mac;
> >> + size_t len;
> >> int ret;
> >>
> >> - if (!pdev)
> >> - return -ENODEV;
> >> + /* Try lookup by device first, there might be a
> >> nvmem_cell_lookup
> >> + * associated with a given device.
> >> + */
> >> + if (pdev) {
> >> + ret = nvmem_get_mac_address(&pdev->dev, addr);
> >> + put_device(&pdev->dev);
> >> + return ret;
> >> + }
> >> +
> >
> > This smells like the wrong band aid :)
> >
> > Any struct device can contain an OF node pointer these days.
>
> But not all nodes might have an associated device, see DSA for example.
I believe what Ben is saying and what I said earlier is going from dev
-> OF node is right and OF node -> dev is wrong. If you only have an
OF node, then use an of_* function.
> And as the name suggests of_get_mac_address() operates on a node. So
> if a driver calls of_get_mac_address() it should work on the node. What
> is wrong IMHO, is that the ethernet drivers where the corresponding
> board
> has a nvmem_cell_lookup registered is calling of_get_mac_address(node).
> It should rather call eth_get_mac_address(dev) in the first place.
>
> One would need to figure out if there is an actual device (with an
> assiciated of_node), then call eth_get_mac_address(dev) and if there
> isn't a device call of_get_mac_address(node).
Yes, I think we're all in agreement.
> But I don't know if that is easy to figure out. Well, one could start
> with just the device where nvmem_cell_lookup is used. Then we could
> drop the workaround above.
Start with the ones just passing dev.of_node directly:
$ git grep 'of_get_mac_address(.*of_node)'
drivers/net/ethernet/aeroflex/greth.c: addr =
of_get_mac_address(ofdev->dev.of_node);
drivers/net/ethernet/altera/altera_tse_main.c: macaddr =
of_get_mac_address(pdev->dev.of_node);
drivers/net/ethernet/arc/emac_main.c: mac_addr =
of_get_mac_address(dev->of_node);
drivers/net/ethernet/broadcom/bgmac-bcma.c: mac =
of_get_mac_address(bgmac->dev->of_node);
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c: mac =
of_get_mac_address(pdev->dev.of_node);
drivers/net/ethernet/ethoc.c: mac =
of_get_mac_address(pdev->dev.of_node);
drivers/net/ethernet/ezchip/nps_enet.c: mac_addr =
of_get_mac_address(dev->of_node);
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c: mac_addr =
of_get_mac_address(ofdev->dev.of_node);
drivers/net/ethernet/marvell/pxa168_eth.c: mac_addr =
of_get_mac_address(pdev->dev.of_node);
drivers/net/ethernet/marvell/sky2.c: iap =
of_get_mac_address(hw->pdev->dev.of_node);
drivers/net/ethernet/mediatek/mtk_eth_soc.c: mac_addr =
of_get_mac_address(mac->of_node);
drivers/net/ethernet/microchip/lan743x_main.c: mac_addr =
of_get_mac_address(pdev->dev.of_node);
drivers/net/ethernet/qualcomm/qca_spi.c: mac =
of_get_mac_address(spi->dev.of_node);
drivers/net/ethernet/qualcomm/qca_uart.c: mac =
of_get_mac_address(serdev->dev.of_node);
drivers/net/ethernet/wiznet/w5100-spi.c: const void *mac =
of_get_mac_address(spi->dev.of_node);
drivers/net/ethernet/xilinx/xilinx_axienet_main.c: mac_addr =
of_get_mac_address(pdev->dev.of_node);
drivers/net/ethernet/xilinx/xilinx_emaclite.c: mac_address =
of_get_mac_address(ofdev->dev.of_node);
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c: mac_addr =
of_get_mac_address(rt2x00dev->dev->of_node);
drivers/staging/octeon/ethernet.c: mac =
of_get_mac_address(priv->of_node);
drivers/staging/wfx/main.c: macaddr =
of_get_mac_address(wdev->dev->of_node);
net/ethernet/eth.c: addr = of_get_mac_address(dev->of_node);
Then this will find most of the rest:
git grep -W 'of_get_mac_address([a-z]*)'| grep -E '(node|np)'
Rob
^ permalink raw reply
* Re: [PATCH v2] tools: do not include scripts/Kbuild.include
From: Yonghong Song @ 2021-04-16 15:39 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: Song Liu, kvm, Alexei Starovoitov, Paul Mackerras,
linux-kselftest, Shuah Khan, Janosch Frank, Daniel Borkmann,
Christian Borntraeger, John Fastabend, Andrii Nakryiko,
clang-built-linux, KP Singh, Nathan Chancellor, netdev,
Nick Desaulniers, linux-kernel, Paolo Bonzini, bpf, linuxppc-dev,
Martin KaFai Lau
In-Reply-To: <20210416130051.239782-1-masahiroy@kernel.org>
On 4/16/21 6:00 AM, Masahiro Yamada wrote:
> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> scripts/Makefile.compiler"), some kselftests fail to build.
>
> The tools/ directory opted out Kbuild, and went in a different
> direction. They copy any kind of files to the tools/ directory
> in order to do whatever they want in their world.
>
> tools/build/Build.include mimics scripts/Kbuild.include, but some
> tool Makefiles included the Kbuild one to import a feature that is
> missing in tools/build/Build.include:
>
> - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
> only if supported") included scripts/Kbuild.include from
> tools/thermal/tmon/Makefile to import the cc-option macro.
>
> - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
> not support -no-pie") included scripts/Kbuild.include from
> tools/testing/selftests/kvm/Makefile to import the try-run macro.
>
> - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
> failures") included scripts/Kbuild.include from
> tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
> target.
>
> - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
> unrecognized option") included scripts/Kbuild.include from
> tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
> try-run macro.
>
> Copy what they need into tools/build/Build.include, and make them
> include it instead of scripts/Kbuild.include.
>
> Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f45a1@linux.ibm.com/
> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
> Reported-by: Janosch Frank <frankja@linux.ibm.com>
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
LGTM although I see some tools Makefile directly added
".DELETE_ON_ERROR:" in their Makefile.
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH v3] powerpc: fix EDEADLOCK redefinition error in uapi/asm/errno.h
From: Tony Ambardar @ 2021-04-17 1:00 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-arch, Arnd Bergmann, LKML, Stable, Paul Mackerras,
Rosen Penev, bpf, linuxppc-dev
In-Reply-To: <87r1jaeclf.fsf@mpe.ellerman.id.au>
On Fri, 16 Apr 2021 at 03:41, Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Tony Ambardar <tony.ambardar@gmail.com> writes:
> > Hello Michael,
> >
> > The latest version of this patch addressed all feedback I'm aware of
> > when submitted last September, and I've seen no further comments from
> > reviewers since then.
> >
> > Could you please let me know where this stands and if anything further
> > is needed?
>
> Sorry, it's still sitting in my inbox :/
>
> I was going to reply to suggest we split the tools change out. The
> headers under tools are usually updated by another maintainer, I think
> it might even be scripted.
>
> Anyway I've applied your patch and done that (dropped the change to
> tools/.../errno.h), which should also mean the stable backport is more
> likely to work automatically.
>
> It will hit mainline in v5.13-rc1 and then be backported to the stable
> trees.
>
> I don't think you actually need the tools version of the header updated
> to fix your bug? In which case we can probably just wait for it to be
> updated automatically when the tools headers are sync'ed with the kernel
> versions.
>
> cheers
I appreciate the follow up. My original bug was indeed with the tools
header but is being patched locally, so waiting for those headers to
sync with the kernel versions is fine if it simplifies things overall.
Thanks,
Tony
^ permalink raw reply
* Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
From: kernel test robot @ 2021-04-17 1:08 UTC (permalink / raw)
To: Matthew Wilcox (Oracle), brouer
Cc: kbuild-all, netdev, ilias.apalodimas, linux-kernel,
Matthew Wilcox (Oracle), linux-mips, linux-mm, mcroce,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20210416230724.2519198-2-willy@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 2920 bytes --]
Hi "Matthew,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.12-rc7]
[cannot apply to hnaz-linux-mm/master next-20210416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Change-struct-page-layout-for-page_pool/20210417-070951
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5e46d1b78a03d52306f21f77a4e4a144b6d31486
config: parisc-randconfig-s031-20210416 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-280-g2cd6d34e-dirty
# https://github.com/0day-ci/linux/commit/898e155048088be20b2606575a24108eacc4c91b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Change-struct-page-layout-for-page_pool/20210417-070951
git checkout 898e155048088be20b2606575a24108eacc4c91b
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=parisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from net/core/xdp.c:15:
include/net/page_pool.h: In function 'page_pool_get_dma_addr':
>> include/net/page_pool.h:203:40: warning: left shift count >= width of type [-Wshift-count-overflow]
203 | ret |= (dma_addr_t)page->dma_addr[1] << 32;
| ^~
include/net/page_pool.h: In function 'page_pool_set_dma_addr':
>> include/net/page_pool.h:211:28: warning: right shift count >= width of type [-Wshift-count-overflow]
211 | page->dma_addr[1] = addr >> 32;
| ^~
vim +203 include/net/page_pool.h
198
199 static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
200 {
201 dma_addr_t ret = page->dma_addr[0];
202 if (sizeof(dma_addr_t) > sizeof(unsigned long))
> 203 ret |= (dma_addr_t)page->dma_addr[1] << 32;
204 return ret;
205 }
206
207 static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
208 {
209 page->dma_addr[0] = addr;
210 if (sizeof(dma_addr_t) > sizeof(unsigned long))
> 211 page->dma_addr[1] = addr >> 32;
212 }
213
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23601 bytes --]
^ permalink raw reply
* Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings
From: Nicholas Piggin @ 2021-04-17 2:39 UTC (permalink / raw)
To: Andrew Morton, Christophe Leroy
Cc: linux-arch, Stephen Rothwell, Ding Tianhong, linux-kernel,
Christoph Hellwig, linux-mm, Jonathan Cameron, Rick, Edgecombe,
linuxppc-dev
In-Reply-To: <20210415115529.9703ba8e9f7a38dea39efa56@linux-foundation.org>
Excerpts from Andrew Morton's message of April 16, 2021 4:55 am:
> On Thu, 15 Apr 2021 12:23:55 +0200 Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>> > + * is done. STRICT_MODULE_RWX may require extra work to support this
>> > + * too.
>> > + */
>> >
>> > - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
>> > - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>>
>>
>> I think you should add the following in <asm/pgtable.h>
>>
>> #ifndef MODULES_VADDR
>> #define MODULES_VADDR VMALLOC_START
>> #define MODULES_END VMALLOC_END
>> #endif
>>
>> And leave module_alloc() as is (just removing the enclosing #ifdef MODULES_VADDR and adding the
>> VM_NO_HUGE_VMAP flag)
>>
>> This would minimise the conflits with the changes I did in powerpc/next reported by Stephen R.
>>
>
> I'll drop powerpc-64s-radix-enable-huge-vmalloc-mappings.patch for now,
> make life simpler.
Yeah that's fine.
> Nick, a redo on top of Christophe's changes in linux-next would be best
> please.
Will do.
Thanks,
Nick
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox