* [PATCH] scsi: ibmvfc: Stop using scsi_cmnd.tag
From: John Garry @ 2021-08-17 13:43 UTC (permalink / raw)
To: tyreld, mpe, benh, paulus, jejb, martin.petersen
Cc: sfr, bvanassche, linux-scsi, John Garry, linux-kernel, linux-next,
hare, linuxppc-dev, hch
Use scsi_cmd_to_rq(scsi_cmnd)->tag in preference to scsi_cmnd.tag.
Signed-off-by: John Garry <john.garry@huawei.com>
---
This patch was missed in a series to remove scsi_cmnd.tag, which caused
a build error on Martin's SCSI staging tree:
https://lore.kernel.org/linux-scsi/yq14kbppa42.fsf@ca-mkp.ca.oracle.com/T/#mb47909f38f35837686734369600051b278d124af
I note that scsi_cmnd.tag is/was an unsigned char, and I could not find
anywhere in the driver which limits scsi_host.can_queue to 255, so using
scsi_cmnd.tag looks odd to me.
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 7fa5e64e38c3..ba7150cb226a 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1956,7 +1956,7 @@ static int ibmvfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
memcpy(iu->cdb, cmnd->cmnd, cmnd->cmd_len);
if (cmnd->flags & SCMD_TAGGED) {
- vfc_cmd->task_tag = cpu_to_be64(cmnd->tag);
+ vfc_cmd->task_tag = cpu_to_be64(scsi_cmd_to_rq(cmnd)->tag);
iu->pri_task_attr = IBMVFC_SIMPLE_TASK;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v1 2/4] powerpc/64s/perf: add power_pmu_running to query whether perf is being used
From: Michael Ellerman @ 2021-08-17 13:42 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Athira Rajeev, Madhavan Srinivasan, Nicholas Piggin
In-Reply-To: <20210816072953.1165964-3-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Interrupt handling code would like to know whether perf is enabled, to
> know whether it should enable MSR[EE] to improve PMI coverage.
>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/hw_irq.h | 2 ++
> arch/powerpc/perf/core-book3s.c | 13 +++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 21cc571ea9c2..2d5c0d3ccbb6 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -306,6 +306,8 @@ static inline bool lazy_irq_pending_nocheck(void)
> return __lazy_irq_pending(local_paca->irq_happened);
> }
>
> +bool power_pmu_running(void);
> +
> /*
> * This is called by asynchronous interrupts to conditionally
> * re-enable hard interrupts after having cleared the source
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index bb0ee716de91..76114a9afb2b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2380,6 +2380,19 @@ static void perf_event_interrupt(struct pt_regs *regs)
> perf_sample_event_took(sched_clock() - start_clock);
> }
>
> +bool power_pmu_running(void)
> +{
> + struct cpu_hw_events *cpuhw;
> +
> + /* Could this simply test local_paca->pmcregs_in_use? */
I think it could, except that it's under an ifdef:
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
u8 pmcregs_in_use; /* pseries puts this in lppaca */
#endif
cheers
^ permalink raw reply
* [PATCH] powerpc/mm: Fix set_memory_*() against concurrent accesses
From: Michael Ellerman @ 2021-08-17 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: lvivier, farosas, jniethe5, npiggin, aneesh.kumar
Laurent reported that STRICT_MODULE_RWX was causing intermittent crashes
on one of his systems:
kernel tried to execute exec-protected page (c008000004073278) - exploit attempt? (uid: 0)
BUG: Unable to handle kernel instruction fetch
Faulting instruction address: 0xc008000004073278
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: drm virtio_console fuse drm_panel_orientation_quirks ...
CPU: 3 PID: 44 Comm: kworker/3:1 Not tainted 5.14.0-rc4+ #12
Workqueue: events control_work_handler [virtio_console]
NIP: c008000004073278 LR: c008000004073278 CTR: c0000000001e9de0
REGS: c00000002e4ef7e0 TRAP: 0400 Not tainted (5.14.0-rc4+)
MSR: 800000004280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 24002822 XER: 200400cf
...
NIP fill_queue+0xf0/0x210 [virtio_console]
LR fill_queue+0xf0/0x210 [virtio_console]
Call Trace:
fill_queue+0xb4/0x210 [virtio_console] (unreliable)
add_port+0x1a8/0x470 [virtio_console]
control_work_handler+0xbc/0x1e8 [virtio_console]
process_one_work+0x290/0x590
worker_thread+0x88/0x620
kthread+0x194/0x1a0
ret_from_kernel_thread+0x5c/0x64
Jordan, Fabiano & Murilo were able to reproduce and identify that the
problem is caused by the call to module_enable_ro() in do_init_module(),
which happens after the module's init function has already been called.
Our current implementation of change_page_attr() is not safe against
concurrent accesses, because it invalidates the PTE before flushing the
TLB and then installing the new PTE. That leaves a window in time where
there is no valid PTE for the page, if another CPU tries to access the
page at that time we see something like the fault above.
We can't simply switch to set_pte_at()/flush TLB, because our hash MMU
code doesn't handle a set_pte_at() of a valid PTE. See [1].
But we do have pte_update(), which replaces the old PTE with the new,
meaning there's no window where the PTE is invalid. And the hash MMU
version hash__pte_update() deals with synchronising the hash page table
correctly.
Because pte_update() takes the set of PTE bits to set and clear we can't
use our existing helpers, eg. pte_wrprotect() etc. and instead have to
open code the set of flags. We will clean that up somehow in a future
commit.
[1]: https://lore.kernel.org/linuxppc-dev/87y318wp9r.fsf@linux.ibm.com/
Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
Reported-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/pageattr.c | 45 +++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index 0876216ceee6..72425b61eb7e 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -18,52 +18,61 @@
/*
* Updates the attributes of a page in three steps:
*
- * 1. invalidate the page table entry
- * 2. flush the TLB
- * 3. install the new entry with the updated attributes
- *
- * Invalidating the pte means there are situations where this will not work
- * when in theory it should.
- * For example:
- * - removing write from page whilst it is being executed
- * - setting a page read-only whilst it is being read by another CPU
+ * 1. take the page_table_lock
+ * 2. install the new entry with the updated attributes
+ * 3. flush the TLB
*
+ * This sequence is safe against concurrent updates, and also allows updating the
+ * attributes of a page currently being executed or accessed.
*/
static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
{
long action = (long)data;
- pte_t pte;
+ unsigned long set, clear;
spin_lock(&init_mm.page_table_lock);
- /* invalidate the PTE so it's safe to modify */
- pte = ptep_get_and_clear(&init_mm, addr, ptep);
- flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+ set = clear = 0;
/* modify the PTE bits as desired, then apply */
switch (action) {
case SET_MEMORY_RO:
- pte = pte_wrprotect(pte);
+#ifdef CONFIG_PPC_BOOK3S_64
+ clear = _PAGE_WRITE;
+#elif defined(CONFIG_PPC_8xx)
+ set = _PAGE_RO;
+#else
+ clear = _PAGE_RW;
+#endif
break;
case SET_MEMORY_RW:
- pte = pte_mkwrite(pte_mkdirty(pte));
+#ifdef CONFIG_PPC_8xx
+ clear = _PAGE_RO;
+#elif defined(CONFIG_PPC_BOOK3S_64)
+ set = _PAGE_RW | _PAGE_DIRTY | _PAGE_SOFT_DIRTY;
+#else
+ set = _PAGE_RW | _PAGE_DIRTY;
+#endif
break;
case SET_MEMORY_NX:
- pte = pte_exprotect(pte);
+ clear = _PAGE_EXEC;
break;
case SET_MEMORY_X:
- pte = pte_mkexec(pte);
+ set = _PAGE_EXEC;
break;
default:
WARN_ON_ONCE(1);
break;
}
- set_pte_at(&init_mm, addr, ptep, pte);
+ pte_update(&init_mm, addr, ptep, clear, set, 0);
/* See ptesync comment in radix__set_pte_at() */
if (radix_enabled())
asm volatile("ptesync": : :"memory");
+
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
spin_unlock(&init_mm.page_table_lock);
return 0;
base-commit: cbc06f051c524dcfe52ef0d1f30647828e226d30
--
2.25.1
^ permalink raw reply related
* [PATCH v7 2/2] virtio-console: remove unnecessary kmemdup()
From: Xianting Tian @ 2021-08-17 13:23 UTC (permalink / raw)
To: gregkh, jirislaby, amit, arnd, osandov
Cc: Xianting Tian, linuxppc-dev, linux-kernel, virtualization
In-Reply-To: <20210817132300.165014-1-xianting.tian@linux.alibaba.com>
hvc framework will never pass stack memory to the put_chars() function,
So the calling of kmemdup() is unnecessary, we can remove it.
This revert commit c4baad5029 ("virtio-console: avoid DMA from stack")
Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
---
drivers/char/virtio_console.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7eaf303a7..4ed3ffb1d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1117,8 +1117,6 @@ static int put_chars(u32 vtermno, const char *buf, int count)
{
struct port *port;
struct scatterlist sg[1];
- void *data;
- int ret;
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
@@ -1127,14 +1125,8 @@ static int put_chars(u32 vtermno, const char *buf, int count)
if (!port)
return -EPIPE;
- data = kmemdup(buf, count, GFP_ATOMIC);
- if (!data)
- return -ENOMEM;
-
- sg_init_one(sg, data, count);
- ret = __send_to_port(port, sg, 1, count, data, false);
- kfree(data);
- return ret;
+ sg_init_one(sg, buf, count);
+ return __send_to_port(port, sg, 1, count, (void *)buf, false);
}
/*
--
2.17.1
^ permalink raw reply related
* [PATCH v7 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Xianting Tian @ 2021-08-17 13:22 UTC (permalink / raw)
To: gregkh, jirislaby, amit, arnd, osandov
Cc: Xianting Tian, linuxppc-dev, linux-kernel, virtualization
In-Reply-To: <20210817132300.165014-1-xianting.tian@linux.alibaba.com>
As well known, hvc backend can register its opertions to hvc backend.
the opertions contain put_chars(), get_chars() and so on.
Some hvc backend may do dma in its opertions. eg, put_chars() of
virtio-console. But in the code of hvc framework, it may pass DMA
incapable memory to put_chars() under a specific configuration, which
is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
1, c[] is on stack,
hvc_console_print():
char c[N_OUTBUF] __ALIGNED__;
cons_ops[index]->put_chars(vtermnos[index], c, i);
2, ch is on stack,
static void hvc_poll_put_char(,,char ch)
{
struct tty_struct *tty = driver->ttys[0];
struct hvc_struct *hp = tty->driver_data;
int n;
do {
n = hp->ops->put_chars(hp->vtermno, &ch, 1);
} while (n <= 0);
}
Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
is passed to virtio-console by hvc framework in above code. But I think
the fix is aggressive, it directly uses kmemdup() to alloc new buffer
from kmalloc area and do memcpy no matter the memory is in kmalloc area
or not. But most importantly, it should better be fixed in the hvc
framework, by changing it to never pass stack memory to the put_chars()
function in the first place. Otherwise, we still face the same issue if
a new hvc backend using dma added in the furture.
We make 'char c[N_OUTBUF]' part of 'struct hvc_struct', so hp->c is no
longer the stack memory. we can use it in above two cases.
Other fix is use L1_CACHE_BYTES as the alignment, use 'sizeof(long)' as
dma alignment is wrong. And use struct_size() to calculate size of
hvc_struct.
Introduce another array(cons_hvcs[]) for hvc_struct pointers next to
the cons_ops[] and vtermnos[] arrays. With the array, we can easily find
the hp, instead of traversing hp list.
With the patch, we can remove the fix c4baad5029.
We tested the patch, it worked normally.
Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
---
drivers/tty/hvc/hvc_console.c | 39 +++++++++++++++++++++--------------
drivers/tty/hvc/hvc_console.h | 16 ++++++++++++--
2 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 5bb8c4e44..b882ceb5f 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -41,16 +41,6 @@
*/
#define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
-/*
- * These sizes are most efficient for vio, because they are the
- * native transfer size. We could make them selectable in the
- * future to better deal with backends that want other buffer sizes.
- */
-#define N_OUTBUF 16
-#define N_INBUF 16
-
-#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
-
static struct tty_driver *hvc_driver;
static struct task_struct *hvc_task;
@@ -142,6 +132,7 @@ static int hvc_flush(struct hvc_struct *hp)
static const struct hv_ops *cons_ops[MAX_NR_HVC_CONSOLES];
static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
{[0 ... MAX_NR_HVC_CONSOLES - 1] = -1};
+static struct hvc_struct *cons_hvcs[MAX_NR_HVC_CONSOLES];
/*
* Console APIs, NOT TTY. These APIs are available immediately when
@@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
static void hvc_console_print(struct console *co, const char *b,
unsigned count)
{
- char c[N_OUTBUF] __ALIGNED__;
+ char *c;
unsigned i = 0, n = 0;
int r, donecr = 0, index = co->index;
+ unsigned long flags;
+ struct hvc_struct *hp;
/* Console access attempt outside of acceptable console range. */
if (index >= MAX_NR_HVC_CONSOLES)
@@ -163,6 +156,13 @@ static void hvc_console_print(struct console *co, const char *b,
if (vtermnos[index] == -1)
return;
+ hp = cons_hvcs[index];
+ if (!hp || !hp->c)
+ return;
+
+ c = hp->c;
+
+ spin_lock_irqsave(&hp->c_lock, flags);
while (count > 0 || i > 0) {
if (count > 0 && i < sizeof(c)) {
if (b[n] == '\n' && !donecr) {
@@ -191,6 +191,7 @@ static void hvc_console_print(struct console *co, const char *b,
}
}
}
+ spin_unlock_irqrestore(&hp->c_lock, flags);
hvc_console_flush(cons_ops[index], vtermnos[index]);
}
@@ -878,9 +879,16 @@ static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch)
struct tty_struct *tty = driver->ttys[0];
struct hvc_struct *hp = tty->driver_data;
int n;
+ unsigned long flags;
+
+ if (!hp || !hp->c)
+ return;
do {
- n = hp->ops->put_chars(hp->vtermno, &ch, 1);
+ spin_lock_irqsave(&hp->c_lock, flags);
+ hp->c[0] = ch;
+ n = hp->ops->put_chars(hp->vtermno, hp->c, 1);
+ spin_unlock_irqrestore(&hp->c_lock, flags);
} while (n <= 0);
}
#endif
@@ -922,8 +930,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
return ERR_PTR(err);
}
- hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
- GFP_KERNEL);
+ hp = kzalloc(struct_size(hp, outbuf, outbuf_size), GFP_KERNEL);
if (!hp)
return ERR_PTR(-ENOMEM);
@@ -931,13 +938,13 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
hp->data = data;
hp->ops = ops;
hp->outbuf_size = outbuf_size;
- hp->outbuf = &((char *)hp)[ALIGN(sizeof(*hp), sizeof(long))];
tty_port_init(&hp->port);
hp->port.ops = &hvc_port_ops;
INIT_WORK(&hp->tty_resize, hvc_set_winsz);
spin_lock_init(&hp->lock);
+ spin_lock_init(&hp->c_lock);
mutex_lock(&hvc_structs_mutex);
/*
@@ -964,6 +971,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
if (i < MAX_NR_HVC_CONSOLES) {
cons_ops[i] = ops;
vtermnos[i] = vtermno;
+ cons_hvcs[i] = hp;
}
list_add_tail(&(hp->next), &hvc_structs);
@@ -988,6 +996,7 @@ int hvc_remove(struct hvc_struct *hp)
if (hp->index < MAX_NR_HVC_CONSOLES) {
vtermnos[hp->index] = -1;
cons_ops[hp->index] = NULL;
+ cons_hvcs[hp->index] = NULL;
}
/* Don't whack hp->irq because tty_hangup() will need to free the irq. */
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index 18d005814..97a5f1e0f 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -32,13 +32,21 @@
*/
#define HVC_ALLOC_TTY_ADAPTERS 8
+/*
+ * These sizes are most efficient for vio, because they are the
+ * native transfer size. We could make them selectable in the
+ * future to better deal with backends that want other buffer sizes.
+ */
+#define N_OUTBUF 16
+#define N_INBUF 16
+
+#define __ALIGNED__ __attribute__((__aligned__(L1_CACHE_BYTES)))
+
struct hvc_struct {
struct tty_port port;
spinlock_t lock;
int index;
int do_wakeup;
- char *outbuf;
- int outbuf_size;
int n_outbuf;
uint32_t vtermno;
const struct hv_ops *ops;
@@ -48,6 +56,10 @@ struct hvc_struct {
struct work_struct tty_resize;
struct list_head next;
unsigned long flags;
+ spinlock_t c_lock;
+ char c[N_OUTBUF] __ALIGNED__;
+ int outbuf_size;
+ char outbuf[0] __ALIGNED__;
};
/* implemented by a low level driver */
--
2.17.1
^ permalink raw reply related
* [PATCH v7 0/2] make hvc pass dma capable memory to its backend
From: Xianting Tian @ 2021-08-17 13:22 UTC (permalink / raw)
To: gregkh, jirislaby, amit, arnd, osandov
Cc: Xianting Tian, linuxppc-dev, linux-kernel, virtualization
Dear all,
This patch series make hvc framework pass DMA capable memory to
put_chars() of hvc backend(eg, virtio-console), and revert commit
c4baad5029 ("virtio-console: avoid DMA from stack”)
V1
virtio-console: avoid DMA from vmalloc area
https://lkml.org/lkml/2021/7/27/494
For v1 patch, Arnd Bergmann suggests to fix the issue in the first
place:
Make hvc pass DMA capable memory to put_chars()
The fix suggestion is included in v2.
V2
[PATCH 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/1/8
[PATCH 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/1/9
For v2 patch, Arnd Bergmann suggests to make new buf part of the
hvc_struct structure, and fix the compile issue.
The fix suggestion is included in v3.
V3
[PATCH v3 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/3/1347
[PATCH v3 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/3/1348
For v3 patch, Jiri Slaby suggests to make 'char c[N_OUTBUF]' part of
hvc_struct, and make 'hp->outbuf' aligned and use struct_size() to
calculate the size of hvc_struct. The fix suggestion is included in
v4.
V4
[PATCH v4 0/2] make hvc pass dma capable memory to its backend
https://lkml.org/lkml/2021/8/5/1350
[PATCH v4 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/5/1351
[PATCH v4 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/5/1352
For v4 patch, Arnd Bergmann suggests to introduce another
array(cons_outbuf[]) for the buffer pointers next to the cons_ops[]
and vtermnos[] arrays. This fix included in this v5 patch.
V5
Arnd Bergmann suggests to use "L1_CACHE_BYTES" as dma alignment,
use 'sizeof(long)' as dma alignment is wrong. fix it in v6.
V6
It contains coding error, fix it in v7 and it worked normally
according to test result.
drivers/tty/hvc/hvc_console.c | 39 +++++++++++++++++++++--------------
drivers/tty/hvc/hvc_console.h | 16 ++++++++++++--
drivers/tty/hvc/hvc_console.h | 16 ++++++++++++--
3 file changed
^ permalink raw reply
* Re: [PATCH/RFC] powerpc/module_64: allow .init_array constructors to run
From: Christophe Leroy @ 2021-08-17 13:22 UTC (permalink / raw)
To: Jan Stancek, mpe, benh, paulus, linuxppc-dev; +Cc: linux-kernel
In-Reply-To: <920acea9aa18e4f2956581a8e158bdaa376fdf63.1629203945.git.jstancek@redhat.com>
Le 17/08/2021 à 15:02, Jan Stancek a écrit :
> gcov and kasan rely on compiler generated constructor code.
> For modules, gcc-8 with gcov enabled generates .init_array section,
> but on ppc64le it doesn't get executed. find_module_sections() never
> finds .init_array section, because module_frob_arch_sections() renames
> it to _init_array.
>
> Avoid renaming .init_array section, so do_mod_ctors() can use it.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> I wasn't able to trace the comment:
> "We don't handle .init for the moment: rename to _init"
Original patch there: https://github.com/mpe/linux-fullhistory/commit/d6ad6690aa72
> to original patch (it pre-dates .git). I'm not sure if it
> still applies today, so I limited patch to .init_array. This
> fixes gcov for modules for me on ppc64le 5.14.0-rc6.
>
> Renaming issue is also mentioned in kasan patches here:
> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20210319144058.772525-1-dja@axtens
>
> arch/powerpc/kernel/module_64.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 6baa676e7cb6..c604b13ea6bf 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -299,8 +299,16 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
> sechdrs[i].sh_size);
>
> /* We don't handle .init for the moment: rename to _init */
> - while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
> + while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init"))) {
> +#ifdef CONFIG_CONSTRUCTORS
Please avoid #ifdefery as much as possible.
I think here you can do:
if (IS_ENABLED(CONFIG_CONSTRUCTORS) &&
strstr(secstrings + sechdrs[i].sh_name, ".init_array"))
> + /* find_module_sections() needs .init_array intact */
> + if (strstr(secstrings + sechdrs[i].sh_name,
> + ".init_array")) {
No { } for single statements (See
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces)
> + break;
> + }
> +#endif
> p[0] = '_';
> + }
>
> if (sechdrs[i].sh_type == SHT_SYMTAB)
> dedotify((void *)hdr + sechdrs[i].sh_offset,
>
^ permalink raw reply
* Re: [PATCH v1 2/4] powerpc/64s/perf: add power_pmu_running to query whether perf is being used
From: Madhavan Srinivasan @ 2021-08-17 13:06 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Athira Rajeev
In-Reply-To: <20210816072953.1165964-3-npiggin@gmail.com>
On 8/16/21 12:59 PM, Nicholas Piggin wrote:
> Interrupt handling code would like to know whether perf is enabled, to
> know whether it should enable MSR[EE] to improve PMI coverage.
>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/hw_irq.h | 2 ++
> arch/powerpc/perf/core-book3s.c | 13 +++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 21cc571ea9c2..2d5c0d3ccbb6 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -306,6 +306,8 @@ static inline bool lazy_irq_pending_nocheck(void)
> return __lazy_irq_pending(local_paca->irq_happened);
> }
>
> +bool power_pmu_running(void);
> +
> /*
> * This is called by asynchronous interrupts to conditionally
> * re-enable hard interrupts after having cleared the source
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index bb0ee716de91..76114a9afb2b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2380,6 +2380,19 @@ static void perf_event_interrupt(struct pt_regs *regs)
> perf_sample_event_took(sched_clock() - start_clock);
> }
>
> +bool power_pmu_running(void)
> +{
> + struct cpu_hw_events *cpuhw;
> +
> + /* Could this simply test local_paca->pmcregs_in_use? */
> +
> + if (!ppmu)
> + return false;
This covers only when perf platform driver is not registered,
but we should also check for MMCR0[32], since pmu sprs can be
accessed via sysfs.
Maddy
> +
> + cpuhw = this_cpu_ptr(&cpu_hw_events);
> + return cpuhw->n_events;
> +}
> +
> static int power_pmu_prepare_cpu(unsigned int cpu)
> {
> struct cpu_hw_events *cpuhw = &per_cpu(cpu_hw_events, cpu);
^ permalink raw reply
* [PATCH/RFC] powerpc/module_64: allow .init_array constructors to run
From: Jan Stancek @ 2021-08-17 13:02 UTC (permalink / raw)
To: mpe, benh, paulus, christophe.leroy, linuxppc-dev; +Cc: linux-kernel, jstancek
gcov and kasan rely on compiler generated constructor code.
For modules, gcc-8 with gcov enabled generates .init_array section,
but on ppc64le it doesn't get executed. find_module_sections() never
finds .init_array section, because module_frob_arch_sections() renames
it to _init_array.
Avoid renaming .init_array section, so do_mod_ctors() can use it.
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
I wasn't able to trace the comment:
"We don't handle .init for the moment: rename to _init"
to original patch (it pre-dates .git). I'm not sure if it
still applies today, so I limited patch to .init_array. This
fixes gcov for modules for me on ppc64le 5.14.0-rc6.
Renaming issue is also mentioned in kasan patches here:
https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20210319144058.772525-1-dja@axtens
arch/powerpc/kernel/module_64.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..c604b13ea6bf 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -299,8 +299,16 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
sechdrs[i].sh_size);
/* We don't handle .init for the moment: rename to _init */
- while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
+ while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init"))) {
+#ifdef CONFIG_CONSTRUCTORS
+ /* find_module_sections() needs .init_array intact */
+ if (strstr(secstrings + sechdrs[i].sh_name,
+ ".init_array")) {
+ break;
+ }
+#endif
p[0] = '_';
+ }
if (sechdrs[i].sh_type == SHT_SYMTAB)
dedotify((void *)hdr + sechdrs[i].sh_offset,
--
2.27.0
^ permalink raw reply related
* [PATCH] powerpc/head_check: Fix shellcheck errors
From: Michael Ellerman @ 2021-08-17 12:51 UTC (permalink / raw)
To: linuxppc-dev
Replace "cat file | grep pattern" with "grep pattern file", and quote a
few variables. Together that fixes all shellcheck errors.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/tools/head_check.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/tools/head_check.sh b/arch/powerpc/tools/head_check.sh
index e477837fdc58..689907cda996 100644
--- a/arch/powerpc/tools/head_check.sh
+++ b/arch/powerpc/tools/head_check.sh
@@ -49,11 +49,11 @@ vmlinux="$2"
$nm "$vmlinux" | grep -e " [TA] _stext$" -e " t start_first_256B$" -e " a text_start$" -e " t start_text$" > .tmp_symbols.txt
-vma=$(cat .tmp_symbols.txt | grep -e " [TA] _stext$" | cut -d' ' -f1)
+vma=$(grep -e " [TA] _stext$" .tmp_symbols.txt | cut -d' ' -f1)
-expected_start_head_addr=$vma
+expected_start_head_addr="$vma"
-start_head_addr=$(cat .tmp_symbols.txt | grep " t start_first_256B$" | cut -d' ' -f1)
+start_head_addr=$(grep " t start_first_256B$" .tmp_symbols.txt | cut -d' ' -f1)
if [ "$start_head_addr" != "$expected_start_head_addr" ]; then
echo "ERROR: head code starts at $start_head_addr, should be $expected_start_head_addr" 1>&2
@@ -63,11 +63,11 @@ if [ "$start_head_addr" != "$expected_start_head_addr" ]; then
exit 1
fi
-top_vma=$(echo $vma | cut -d'0' -f1)
+top_vma=$(echo "$vma" | cut -d'0' -f1)
-expected_start_text_addr=$(cat .tmp_symbols.txt | grep " a text_start$" | cut -d' ' -f1 | sed "s/^0/$top_vma/")
+expected_start_text_addr=$(grep " a text_start$" .tmp_symbols.txt | cut -d' ' -f1 | sed "s/^0/$top_vma/")
-start_text_addr=$(cat .tmp_symbols.txt | grep " t start_text$" | cut -d' ' -f1)
+start_text_addr=$(grep " t start_text$" .tmp_symbols.txt | cut -d' ' -f1)
if [ "$start_text_addr" != "$expected_start_text_addr" ]; then
echo "ERROR: start_text address is $start_text_addr, should be $expected_start_text_addr" 1>&2
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: Michael Ellerman @ 2021-08-17 12:49 UTC (permalink / raw)
To: Christophe Leroy, kajoljain, linuxppc-dev
Cc: Sukadev Bhattiprolu, atrajeev, maddy, rnsastry
In-Reply-To: <3a34c79d-b800-1a11-7a4b-1fb3babb9df1@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 16/08/2021 à 08:44, kajoljain a écrit :
>> On 8/14/21 6:14 PM, Michael Ellerman wrote:
...
>>>
>>> eg.
>>>
>>> if (use_siar && siar_valid(regs) && siar)
>>> return siar + perf_ip_adjust(regs);
>>> else if (use_siar)
>>> return 0; // no valid instruction pointer
>>> else
>>> return regs->nip;
>>>
>>>
>>> I'm also not sure why we have that return 0 case, I can't think of why
>>> we'd ever want to do that rather than using nip. So maybe we should do
>>> another patch to drop that case.
>>
>> Yeah make sense. I will remove return 0 case in my next version.
>
> This was added by commit
> https://github.com/linuxppc/linux/commit/e6878835ac4794f25385522d29c634b7bbb7cca9
>
> Are we sure it was an error to add it and it can be removed ?
I think so.
That commit added siar_valid(), and updated record_and_restart() to only
record if siar_valid() returned true.
- record = 1;
+ record = siar_valid(regs);
It then also changed perf_instruction_pointer():
- if (use_siar)
+ if (use_siar && siar_valid(regs))
return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
+ else if (use_siar)
+ return 0; // no valid instruction pointer
else
return regs->nip;
The first change means we would never even call
perf_instruction_pointer() if siar_valid() is false, so we could never
hit the use_siar && !siar_valid() case.
But even so it's always preferable to use regs->nip than 0, even if nip
is somewhat skewed due to interrupts being disabled etc.
cheers
^ permalink raw reply
* Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
From: Michael Ellerman @ 2021-08-17 12:38 UTC (permalink / raw)
To: Tom Lendacky, linux-kernel, x86, linuxppc-dev, linux-s390, iommu,
kvm, linux-efi, platform-driver-x86, linux-graphics-maintainer,
amd-gfx, dri-devel, kexec, linux-fsdevel
Cc: Sathyanarayanan Kuppuswamy, Andi Kleen, Tianyu Lan, Joerg Roedel,
Borislav Petkov, Brijesh Singh, Paul Mackerras
In-Reply-To: <000f627ce20c6504dd8d118d85bd69e7717b752f.1628873970.git.thomas.lendacky@amd.com>
Tom Lendacky <thomas.lendacky@amd.com> writes:
> Introduce a powerpc version of the prot_guest_has() function. This will
> be used to replace the powerpc mem_encrypt_active() implementation, so
> the implementation will initially only support the PATTR_MEM_ENCRYPT
> attribute.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/powerpc/include/asm/protected_guest.h | 30 ++++++++++++++++++++++
> arch/powerpc/platforms/pseries/Kconfig | 1 +
> 2 files changed, 31 insertions(+)
> create mode 100644 arch/powerpc/include/asm/protected_guest.h
>
> diff --git a/arch/powerpc/include/asm/protected_guest.h b/arch/powerpc/include/asm/protected_guest.h
> new file mode 100644
> index 000000000000..ce55c2c7e534
> --- /dev/null
> +++ b/arch/powerpc/include/asm/protected_guest.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Protected Guest (and Host) Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky <thomas.lendacky@amd.com>
> + */
> +
> +#ifndef _POWERPC_PROTECTED_GUEST_H
> +#define _POWERPC_PROTECTED_GUEST_H
Minor nit, we would usually use _ASM_POWERPC_PROTECTED_GUEST_H
Otherwise looks OK to me.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH] macintosh: no need to initilise statics to 0
From: Christophe Leroy @ 2021-08-17 11:59 UTC (permalink / raw)
To: Jason Wang, benh; +Cc: yukuai3, linuxppc-dev, linux-kernel
In-Reply-To: <20210817115104.30057-1-wangborong@cdjrlc.com>
Le 17/08/2021 à 13:51, Jason Wang a écrit :
> Global static variables dont need to be initialised to 0. Because
> the compiler will initilise them.
It is not the compiler, it is the Kernel. It is done here:
https://elixir.bootlin.com/linux/v5.14-rc6/source/arch/powerpc/kernel/early_32.c
>
> Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
> ---
> drivers/macintosh/mediabay.c | 10 ++---
> drivers/macintosh/via-pmu.c | 78 ++++++++++++++++++------------------
> 2 files changed, 44 insertions(+), 44 deletions(-)
Among those 44 changes, only 2 are related to the commit's description.
>
> diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
> index eab7e83c11c4..e104a95decb5 100644
> --- a/drivers/macintosh/mediabay.c
> +++ b/drivers/macintosh/mediabay.c
> @@ -70,7 +70,7 @@ struct media_bay_info {
> #define MAX_BAYS 2
>
> static struct media_bay_info media_bays[MAX_BAYS];
> -static int media_bay_count = 0;
> +static int media_bay_count;
>
> /*
> * Wait that number of ms between each step in normal polling mode
> @@ -129,7 +129,7 @@ enum {
> /*
> * Functions for polling content of media bay
> */
> -
> +
What is that change ? Unrelated.
> static u8
> ohare_mb_content(struct media_bay_info *bay)
> {
> @@ -336,7 +336,7 @@ static inline void set_mb_power(struct media_bay_info* bay, int onoff)
> bay->ops->power(bay, 1);
> bay->state = mb_powering_up;
> pr_debug("mediabay%d: powering up\n", bay->index);
> - } else {
> + } else {
What is that change ? Unrelated.
> /* Make sure everything is powered down & disabled */
> bay->ops->power(bay, 0);
> bay->state = mb_powering_down;
> @@ -577,7 +577,7 @@ static int media_bay_attach(struct macio_dev *mdev,
> macio_release_resources(mdev);
> return -ENOMEM;
> }
> -
> +
What is that change ? Unrelated.
> i = media_bay_count++;
> bay = &media_bays[i];
> bay->mdev = mdev;
> @@ -745,7 +745,7 @@ static int __init media_bay_init(void)
> if (!machine_is(powermac))
> return 0;
>
> - macio_register_driver(&media_bay_driver);
> + macio_register_driver(&media_bay_driver);
What is that change ? Unrelated.
>
> return 0;
> }
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 4bdd4c45e7a7..cba4c2464dbf 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -158,7 +158,7 @@ static struct device_node *vias;
> static struct device_node *gpio_node;
> #endif
> static unsigned char __iomem *gpio_reg;
> -static int gpio_irq = 0;
> +static int gpio_irq;
> static int gpio_irq_enabled = -1;
> static volatile int pmu_suspended;
> static spinlock_t pmu_lock;
> @@ -313,7 +313,7 @@ int __init find_via_pmu(void)
> PMU_INT_SNDBRT |
> PMU_INT_ADB |
> PMU_INT_TICK;
> -
> +
What is that change ? Unrelated.
> if (of_node_name_eq(vias->parent, "ohare") ||
> of_device_is_compatible(vias->parent, "ohare"))
> pmu_kind = PMU_OHARE_BASED;
> @@ -336,7 +336,7 @@ int __init find_via_pmu(void)
> PMU_INT_ADB |
> PMU_INT_TICK |
> PMU_INT_ENVIRONMENT;
> -
> +
What is that change ? Unrelated.
> gpiop = of_find_node_by_name(NULL, "gpio");
> if (gpiop) {
> reg = of_get_property(gpiop, "reg", NULL);
> @@ -358,7 +358,7 @@ int __init find_via_pmu(void)
> printk(KERN_ERR "via-pmu: Can't map address !\n");
> goto fail_via_remap;
> }
> -
> +
What is that change ? Unrelated.
> out_8(&via1[IER], IER_CLR | 0x7f); /* disable all intrs */
> out_8(&via1[IFR], 0x7f); /* clear IFR */
>
> @@ -368,7 +368,7 @@ int __init find_via_pmu(void)
> goto fail_init;
>
> sys_ctrler = SYS_CTRLER_PMU;
> -
> +
What is that change ? Unrelated.
> return 1;
>
> fail_init:
> @@ -623,7 +623,7 @@ init_pmu(void)
> pmu_wait_complete(&req);
> if (req.reply_len > 0)
> pmu_version = req.reply[0];
> -
> +
What is that change ? Unrelated.
> /* Read server mode setting */
> if (pmu_kind == PMU_KEYLARGO_BASED) {
> pmu_request(&req, NULL, 2, PMU_POWER_EVENTS,
> @@ -664,11 +664,11 @@ static void pmu_set_server_mode(int server_mode)
> if (server_mode)
> pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
> PMU_PWR_SET_POWERUP_EVENTS,
> - req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
> + req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
What is that change ? Unrelated.
> else
> pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
> PMU_PWR_CLR_POWERUP_EVENTS,
> - req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
> + req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
What is that change ? Unrelated.
> pmu_wait_complete(&req);
> }
>
> @@ -684,8 +684,8 @@ done_battery_state_ohare(struct adb_request* req)
> * 0x01 : AC indicator
> * 0x02 : charging
> * 0x04 : battery exist
> - * 0x08 :
> - * 0x10 :
> + * 0x08 :
> + * 0x10 :
What is that change ? Unrelated.
> * 0x20 : full charged
> * 0x40 : pcharge reset
> * 0x80 : battery exist
> @@ -708,7 +708,7 @@ done_battery_state_ohare(struct adb_request* req)
> pmu_power_flags |= PMU_PWR_AC_PRESENT;
> else
> pmu_power_flags &= ~PMU_PWR_AC_PRESENT;
> -
> +
What is that change ? Unrelated.
> if (mb == PMAC_TYPE_COMET) {
> vmax_charged = 189;
> vmax_charging = 213;
> @@ -771,26 +771,26 @@ done_battery_state_smart(struct adb_request* req)
> /* format:
> * [0] : format of this structure (known: 3,4,5)
> * [1] : flags
> - *
> + *
What is that change ? Unrelated.
> * format 3 & 4:
> - *
> + *
What is that change ? Unrelated.
> * [2] : charge
> * [3] : max charge
> * [4] : current
> * [5] : voltage
> - *
> + *
What is that change ? Unrelated.
> * format 5:
> - *
> + *
What is that change ? Unrelated.
> * [2][3] : charge
> * [4][5] : max charge
> * [6][7] : current
> * [8][9] : voltage
> */
> -
> +
What is that change ? Unrelated.
> unsigned int bat_flags = PMU_BATT_TYPE_SMART;
> int amperage;
> unsigned int capa, max, voltage;
> -
> +
What is that change ? Unrelated.
> if (req->reply[1] & 0x01)
> pmu_power_flags |= PMU_PWR_AC_PRESENT;
> else
> @@ -798,7 +798,7 @@ done_battery_state_smart(struct adb_request* req)
>
>
> capa = max = amperage = voltage = 0;
> -
> +
What is that change ? Unrelated.
> if (req->reply[1] & 0x04) {
> bat_flags |= PMU_BATT_PRESENT;
> switch(req->reply[0]) {
> @@ -897,7 +897,7 @@ static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> static int pmu_battery_proc_show(struct seq_file *m, void *v)
> {
> long batnum = (long)m->private;
> -
> +
What is that change ? Unrelated.
> seq_putc(m, '\n');
> seq_printf(m, "flags : %08x\n", pmu_batteries[batnum].flags);
> seq_printf(m, "charge : %d\n", pmu_batteries[batnum].charge);
> @@ -932,7 +932,7 @@ static ssize_t pmu_options_proc_write(struct file *file,
> char tmp[33];
> char *label, *val;
> size_t fcount = count;
> -
> +
What is that change ? Unrelated.
> if (!count)
> return -EINVAL;
> if (count > 32)
> @@ -1304,7 +1304,7 @@ pmu_suspend(void)
>
> if (pmu_state == uninitialized)
> return;
> -
> +
What is that change ? Unrelated.
> spin_lock_irqsave(&pmu_lock, flags);
> pmu_suspended++;
> if (pmu_suspended > 1) {
> @@ -1430,7 +1430,7 @@ pmu_handle_data(unsigned char *data, int len)
> && data[1] == 0x2c && data[3] == 0xff
> && (data[2] & ~1) == 0xf4))
> adb_input(data+1, len-1, 1);
> -#endif /* CONFIG_ADB */
> +#endif /* CONFIG_ADB */
What is that change ? Unrelated.
> }
> break;
>
> @@ -1554,7 +1554,7 @@ pmu_sr_intr(void)
> interrupt_data_len[int_data_last] = data_len;
> } else {
> req = current_req;
> - /*
> + /*
What is that change ? Unrelated.
> * For PMU sleep and freq change requests, we lock the
> * PMU until it's explicitly unlocked. This avoids any
> * spurrious event polling getting in
> @@ -1589,7 +1589,7 @@ via_pmu_interrupt(int irq, void *arg)
> /* This is a bit brutal, we can probably do better */
> spin_lock_irqsave(&pmu_lock, flags);
> ++disable_poll;
> -
> +
What is that change ? Unrelated.
> for (;;) {
> /* On 68k Macs, VIA interrupts are dispatched individually.
> * Unless we are polling, the relevant IRQ flag has already
> @@ -1653,7 +1653,7 @@ via_pmu_interrupt(int irq, void *arg)
> } else if (current_req)
> pmu_start();
> }
> -no_free_slot:
> +no_free_slot:
What is that change ? Unrelated.
> /* Mark the oldest buffer for flushing */
> if (int_data_state[!int_data_last] == int_data_ready) {
> int_data_state[!int_data_last] = int_data_flush;
> @@ -1670,7 +1670,7 @@ via_pmu_interrupt(int irq, void *arg)
> pmu_done(req);
> req = NULL;
> }
> -
> +
What is that change ? Unrelated.
> /* Deal with interrupt datas outside of the lock */
> if (int_data >= 0) {
> pmu_handle_data(interrupt_data[int_data], interrupt_data_len[int_data]);
> @@ -1776,7 +1776,7 @@ pmu_restart(void)
> local_irq_disable();
>
> drop_interrupts = 1;
> -
> +
What is that change ? Unrelated.
> if (pmu_kind != PMU_KEYLARGO_BASED) {
> pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, PMU_INT_ADB |
> PMU_INT_TICK );
> @@ -1830,7 +1830,7 @@ pmu_present(void)
> /*
> * Put the powerbook to sleep.
> */
> -
> +
What is that change ? Unrelated.
> static u32 save_via[8];
> static int __fake_sleep;
>
> @@ -1901,7 +1901,7 @@ static int powerbook_sleep_grackle(void)
>
> pci_read_config_word(grackle, 0x70, &pmcr1);
> /* Apparently, MacOS uses NAP mode for Grackle ??? */
> - pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);
> + pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);
What is that change ? Unrelated.
> pmcr1 |= GRACKLE_PM|GRACKLE_NAP;
> pci_write_config_word(grackle, 0x70, pmcr1);
>
> @@ -1913,7 +1913,7 @@ static int powerbook_sleep_grackle(void)
>
> /* We're awake again, stop grackle PM */
> pci_read_config_word(grackle, 0x70, &pmcr1);
> - pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);
> + pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);
What is that change ? Unrelated.
> pci_write_config_word(grackle, 0x70, pmcr1);
>
> pci_dev_put(grackle);
> @@ -1921,11 +1921,11 @@ static int powerbook_sleep_grackle(void)
> /* Make sure the PMU is idle */
> pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,0);
> restore_via_state();
> -
> +
What is that change ? Unrelated.
> /* Restore L2 cache */
> if (save_l2cr != 0xffffffff && (save_l2cr & L2CR_L2E) != 0)
> _set_L2CR(save_l2cr);
> -
> +
What is that change ? Unrelated.
> /* Restore userland MMU context */
> switch_mmu_context(NULL, current->active_mm, NULL);
>
> @@ -1949,7 +1949,7 @@ powerbook_sleep_Core99(void)
> unsigned long save_l2cr;
> unsigned long save_l3cr;
> struct adb_request req;
> -
> +
What is that change ? Unrelated.
> if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0) {
> printk(KERN_ERR "Sleep mode not supported on this machine\n");
> return -ENOSYS;
> @@ -2014,7 +2014,7 @@ powerbook_sleep_Core99(void)
> /* Restore L3 cache */
> if (save_l3cr != 0xffffffff && (save_l3cr & L3CR_L3E) != 0)
> _set_L3CR(save_l3cr);
> -
> +
What is that change ? Unrelated.
> /* Restore userland MMU context */
> switch_mmu_context(NULL, current->active_mm, NULL);
>
> @@ -2173,7 +2173,7 @@ pmu_open(struct inode *inode, struct file *file)
> return 0;
> }
>
> -static ssize_t
> +static ssize_t
What is that change ? Unrelated.
> pmu_read(struct file *file, char __user *buf,
> size_t count, loff_t *ppos)
> {
> @@ -2219,7 +2219,7 @@ pmu_read(struct file *file, char __user *buf,
> __set_current_state(TASK_RUNNING);
> remove_wait_queue(&pp->wait, &wait);
> spin_unlock_irqrestore(&pp->lock, flags);
> -
> +
What is that change ? Unrelated.
> return ret;
> }
>
> @@ -2236,7 +2236,7 @@ pmu_fpoll(struct file *filp, poll_table *wait)
> struct pmu_private *pp = filp->private_data;
> __poll_t mask = 0;
> unsigned long flags;
> -
> +
What is that change ? Unrelated.
> if (!pp)
> return 0;
> poll_wait(filp, &pp->wait, wait);
> @@ -2500,7 +2500,7 @@ device_initcall(pmu_device_init);
>
>
> #ifdef DEBUG_SLEEP
> -static inline void
> +static inline void
What is that change ? Unrelated.
> polled_handshake(void)
> {
> via2[B] &= ~TREQ; eieio();
> @@ -2511,7 +2511,7 @@ polled_handshake(void)
> ;
> }
>
> -static inline void
> +static inline void
What is that change ? Unrelated.
> polled_send_byte(int x)
> {
> via1[ACR] |= SR_OUT | SR_EXT; eieio();
>
^ permalink raw reply
* [PATCH] macintosh: no need to initilise statics to 0
From: Jason Wang @ 2021-08-17 11:51 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, linux-kernel, Jason Wang, yukuai3
Global static variables dont need to be initialised to 0. Because
the compiler will initilise them.
Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
---
drivers/macintosh/mediabay.c | 10 ++---
drivers/macintosh/via-pmu.c | 78 ++++++++++++++++++------------------
2 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
index eab7e83c11c4..e104a95decb5 100644
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -70,7 +70,7 @@ struct media_bay_info {
#define MAX_BAYS 2
static struct media_bay_info media_bays[MAX_BAYS];
-static int media_bay_count = 0;
+static int media_bay_count;
/*
* Wait that number of ms between each step in normal polling mode
@@ -129,7 +129,7 @@ enum {
/*
* Functions for polling content of media bay
*/
-
+
static u8
ohare_mb_content(struct media_bay_info *bay)
{
@@ -336,7 +336,7 @@ static inline void set_mb_power(struct media_bay_info* bay, int onoff)
bay->ops->power(bay, 1);
bay->state = mb_powering_up;
pr_debug("mediabay%d: powering up\n", bay->index);
- } else {
+ } else {
/* Make sure everything is powered down & disabled */
bay->ops->power(bay, 0);
bay->state = mb_powering_down;
@@ -577,7 +577,7 @@ static int media_bay_attach(struct macio_dev *mdev,
macio_release_resources(mdev);
return -ENOMEM;
}
-
+
i = media_bay_count++;
bay = &media_bays[i];
bay->mdev = mdev;
@@ -745,7 +745,7 @@ static int __init media_bay_init(void)
if (!machine_is(powermac))
return 0;
- macio_register_driver(&media_bay_driver);
+ macio_register_driver(&media_bay_driver);
return 0;
}
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 4bdd4c45e7a7..cba4c2464dbf 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -158,7 +158,7 @@ static struct device_node *vias;
static struct device_node *gpio_node;
#endif
static unsigned char __iomem *gpio_reg;
-static int gpio_irq = 0;
+static int gpio_irq;
static int gpio_irq_enabled = -1;
static volatile int pmu_suspended;
static spinlock_t pmu_lock;
@@ -313,7 +313,7 @@ int __init find_via_pmu(void)
PMU_INT_SNDBRT |
PMU_INT_ADB |
PMU_INT_TICK;
-
+
if (of_node_name_eq(vias->parent, "ohare") ||
of_device_is_compatible(vias->parent, "ohare"))
pmu_kind = PMU_OHARE_BASED;
@@ -336,7 +336,7 @@ int __init find_via_pmu(void)
PMU_INT_ADB |
PMU_INT_TICK |
PMU_INT_ENVIRONMENT;
-
+
gpiop = of_find_node_by_name(NULL, "gpio");
if (gpiop) {
reg = of_get_property(gpiop, "reg", NULL);
@@ -358,7 +358,7 @@ int __init find_via_pmu(void)
printk(KERN_ERR "via-pmu: Can't map address !\n");
goto fail_via_remap;
}
-
+
out_8(&via1[IER], IER_CLR | 0x7f); /* disable all intrs */
out_8(&via1[IFR], 0x7f); /* clear IFR */
@@ -368,7 +368,7 @@ int __init find_via_pmu(void)
goto fail_init;
sys_ctrler = SYS_CTRLER_PMU;
-
+
return 1;
fail_init:
@@ -623,7 +623,7 @@ init_pmu(void)
pmu_wait_complete(&req);
if (req.reply_len > 0)
pmu_version = req.reply[0];
-
+
/* Read server mode setting */
if (pmu_kind == PMU_KEYLARGO_BASED) {
pmu_request(&req, NULL, 2, PMU_POWER_EVENTS,
@@ -664,11 +664,11 @@ static void pmu_set_server_mode(int server_mode)
if (server_mode)
pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
PMU_PWR_SET_POWERUP_EVENTS,
- req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
+ req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
else
pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
PMU_PWR_CLR_POWERUP_EVENTS,
- req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
+ req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
pmu_wait_complete(&req);
}
@@ -684,8 +684,8 @@ done_battery_state_ohare(struct adb_request* req)
* 0x01 : AC indicator
* 0x02 : charging
* 0x04 : battery exist
- * 0x08 :
- * 0x10 :
+ * 0x08 :
+ * 0x10 :
* 0x20 : full charged
* 0x40 : pcharge reset
* 0x80 : battery exist
@@ -708,7 +708,7 @@ done_battery_state_ohare(struct adb_request* req)
pmu_power_flags |= PMU_PWR_AC_PRESENT;
else
pmu_power_flags &= ~PMU_PWR_AC_PRESENT;
-
+
if (mb == PMAC_TYPE_COMET) {
vmax_charged = 189;
vmax_charging = 213;
@@ -771,26 +771,26 @@ done_battery_state_smart(struct adb_request* req)
/* format:
* [0] : format of this structure (known: 3,4,5)
* [1] : flags
- *
+ *
* format 3 & 4:
- *
+ *
* [2] : charge
* [3] : max charge
* [4] : current
* [5] : voltage
- *
+ *
* format 5:
- *
+ *
* [2][3] : charge
* [4][5] : max charge
* [6][7] : current
* [8][9] : voltage
*/
-
+
unsigned int bat_flags = PMU_BATT_TYPE_SMART;
int amperage;
unsigned int capa, max, voltage;
-
+
if (req->reply[1] & 0x01)
pmu_power_flags |= PMU_PWR_AC_PRESENT;
else
@@ -798,7 +798,7 @@ done_battery_state_smart(struct adb_request* req)
capa = max = amperage = voltage = 0;
-
+
if (req->reply[1] & 0x04) {
bat_flags |= PMU_BATT_PRESENT;
switch(req->reply[0]) {
@@ -897,7 +897,7 @@ static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
static int pmu_battery_proc_show(struct seq_file *m, void *v)
{
long batnum = (long)m->private;
-
+
seq_putc(m, '\n');
seq_printf(m, "flags : %08x\n", pmu_batteries[batnum].flags);
seq_printf(m, "charge : %d\n", pmu_batteries[batnum].charge);
@@ -932,7 +932,7 @@ static ssize_t pmu_options_proc_write(struct file *file,
char tmp[33];
char *label, *val;
size_t fcount = count;
-
+
if (!count)
return -EINVAL;
if (count > 32)
@@ -1304,7 +1304,7 @@ pmu_suspend(void)
if (pmu_state == uninitialized)
return;
-
+
spin_lock_irqsave(&pmu_lock, flags);
pmu_suspended++;
if (pmu_suspended > 1) {
@@ -1430,7 +1430,7 @@ pmu_handle_data(unsigned char *data, int len)
&& data[1] == 0x2c && data[3] == 0xff
&& (data[2] & ~1) == 0xf4))
adb_input(data+1, len-1, 1);
-#endif /* CONFIG_ADB */
+#endif /* CONFIG_ADB */
}
break;
@@ -1554,7 +1554,7 @@ pmu_sr_intr(void)
interrupt_data_len[int_data_last] = data_len;
} else {
req = current_req;
- /*
+ /*
* For PMU sleep and freq change requests, we lock the
* PMU until it's explicitly unlocked. This avoids any
* spurrious event polling getting in
@@ -1589,7 +1589,7 @@ via_pmu_interrupt(int irq, void *arg)
/* This is a bit brutal, we can probably do better */
spin_lock_irqsave(&pmu_lock, flags);
++disable_poll;
-
+
for (;;) {
/* On 68k Macs, VIA interrupts are dispatched individually.
* Unless we are polling, the relevant IRQ flag has already
@@ -1653,7 +1653,7 @@ via_pmu_interrupt(int irq, void *arg)
} else if (current_req)
pmu_start();
}
-no_free_slot:
+no_free_slot:
/* Mark the oldest buffer for flushing */
if (int_data_state[!int_data_last] == int_data_ready) {
int_data_state[!int_data_last] = int_data_flush;
@@ -1670,7 +1670,7 @@ via_pmu_interrupt(int irq, void *arg)
pmu_done(req);
req = NULL;
}
-
+
/* Deal with interrupt datas outside of the lock */
if (int_data >= 0) {
pmu_handle_data(interrupt_data[int_data], interrupt_data_len[int_data]);
@@ -1776,7 +1776,7 @@ pmu_restart(void)
local_irq_disable();
drop_interrupts = 1;
-
+
if (pmu_kind != PMU_KEYLARGO_BASED) {
pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, PMU_INT_ADB |
PMU_INT_TICK );
@@ -1830,7 +1830,7 @@ pmu_present(void)
/*
* Put the powerbook to sleep.
*/
-
+
static u32 save_via[8];
static int __fake_sleep;
@@ -1901,7 +1901,7 @@ static int powerbook_sleep_grackle(void)
pci_read_config_word(grackle, 0x70, &pmcr1);
/* Apparently, MacOS uses NAP mode for Grackle ??? */
- pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);
+ pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);
pmcr1 |= GRACKLE_PM|GRACKLE_NAP;
pci_write_config_word(grackle, 0x70, pmcr1);
@@ -1913,7 +1913,7 @@ static int powerbook_sleep_grackle(void)
/* We're awake again, stop grackle PM */
pci_read_config_word(grackle, 0x70, &pmcr1);
- pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);
+ pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);
pci_write_config_word(grackle, 0x70, pmcr1);
pci_dev_put(grackle);
@@ -1921,11 +1921,11 @@ static int powerbook_sleep_grackle(void)
/* Make sure the PMU is idle */
pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,0);
restore_via_state();
-
+
/* Restore L2 cache */
if (save_l2cr != 0xffffffff && (save_l2cr & L2CR_L2E) != 0)
_set_L2CR(save_l2cr);
-
+
/* Restore userland MMU context */
switch_mmu_context(NULL, current->active_mm, NULL);
@@ -1949,7 +1949,7 @@ powerbook_sleep_Core99(void)
unsigned long save_l2cr;
unsigned long save_l3cr;
struct adb_request req;
-
+
if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0) {
printk(KERN_ERR "Sleep mode not supported on this machine\n");
return -ENOSYS;
@@ -2014,7 +2014,7 @@ powerbook_sleep_Core99(void)
/* Restore L3 cache */
if (save_l3cr != 0xffffffff && (save_l3cr & L3CR_L3E) != 0)
_set_L3CR(save_l3cr);
-
+
/* Restore userland MMU context */
switch_mmu_context(NULL, current->active_mm, NULL);
@@ -2173,7 +2173,7 @@ pmu_open(struct inode *inode, struct file *file)
return 0;
}
-static ssize_t
+static ssize_t
pmu_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
@@ -2219,7 +2219,7 @@ pmu_read(struct file *file, char __user *buf,
__set_current_state(TASK_RUNNING);
remove_wait_queue(&pp->wait, &wait);
spin_unlock_irqrestore(&pp->lock, flags);
-
+
return ret;
}
@@ -2236,7 +2236,7 @@ pmu_fpoll(struct file *filp, poll_table *wait)
struct pmu_private *pp = filp->private_data;
__poll_t mask = 0;
unsigned long flags;
-
+
if (!pp)
return 0;
poll_wait(filp, &pp->wait, wait);
@@ -2500,7 +2500,7 @@ device_initcall(pmu_device_init);
#ifdef DEBUG_SLEEP
-static inline void
+static inline void
polled_handshake(void)
{
via2[B] &= ~TREQ; eieio();
@@ -2511,7 +2511,7 @@ polled_handshake(void)
;
}
-static inline void
+static inline void
polled_send_byte(int x)
{
via1[ACR] |= SR_OUT | SR_EXT; eieio();
--
2.32.0
^ permalink raw reply related
* Re: [PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function
From: Borislav Petkov @ 2021-08-17 10:24 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
dri-devel, Joerg Roedel, linux-fsdevel, Tianyu Lan, linuxppc-dev
In-Reply-To: <YRuN6QhdIQtlluUh@zn.tnic>
On Tue, Aug 17, 2021 at 12:22:33PM +0200, Borislav Petkov wrote:
> This one wants to be part of the previous patch.
... and the three following patches too - the treewide patch does a
single atomic :) replacement and that's it.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function
From: Borislav Petkov @ 2021-08-17 10:22 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
dri-devel, Joerg Roedel, linux-fsdevel, Tianyu Lan, linuxppc-dev
In-Reply-To: <83e4a62108eec470ac0b3f2510b982794d2b7989.1628873970.git.thomas.lendacky@amd.com>
On Fri, Aug 13, 2021 at 11:59:28AM -0500, Tom Lendacky wrote:
> The mem_encrypt_active() function has been replaced by prot_guest_has(),
> so remove the implementation.
>
> Reviewed-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> include/linux/mem_encrypt.h | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index 5c4a18a91f89..ae4526389261 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -16,10 +16,6 @@
>
> #include <asm/mem_encrypt.h>
>
> -#else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
> -
> -static inline bool mem_encrypt_active(void) { return false; }
> -
> #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */
>
> #ifdef CONFIG_AMD_MEM_ENCRYPT
> --
This one wants to be part of the previous patch.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 07/12] x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
From: Borislav Petkov @ 2021-08-17 10:06 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
dri-devel, linux-fsdevel, Tianyu Lan, Thomas Gleixner,
linuxppc-dev, Ingo Molnar
In-Reply-To: <0b8480d93b5090fcc34cf5d5035d4d89aa765d79.1628873970.git.thomas.lendacky@amd.com>
On Fri, Aug 13, 2021 at 11:59:26AM -0500, Tom Lendacky wrote:
> Replace occurrences of sev_es_active() with the more generic
> prot_guest_has() using PATTR_GUEST_PROT_STATE, except for in
> arch/x86/kernel/sev*.c and arch/x86/mm/mem_encrypt*.c where PATTR_SEV_ES
> will be used. If future support is added for other memory encyrption
> techonologies, the use of PATTR_GUEST_PROT_STATE can be updated, as
> required, to specifically use PATTR_SEV_ES.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/include/asm/mem_encrypt.h | 2 --
> arch/x86/kernel/sev.c | 6 +++---
> arch/x86/mm/mem_encrypt.c | 7 +++----
> arch/x86/realmode/init.c | 3 +--
> 4 files changed, 7 insertions(+), 11 deletions(-)
Same comments to this one as for the previous two.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 06/12] x86/sev: Replace occurrences of sev_active() with prot_guest_has()
From: Borislav Petkov @ 2021-08-17 10:02 UTC (permalink / raw)
To: Tom Lendacky
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
Ard Biesheuvel, linux-s390, Andi Kleen, Joerg Roedel, x86,
amd-gfx, Ingo Molnar, linux-graphics-maintainer, Joerg Roedel,
Tianyu Lan, Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel,
iommu, linux-fsdevel, linuxppc-dev
In-Reply-To: <2b3a8fc4659f2e7617399cecdcca549e0fa1dcb7.1628873970.git.thomas.lendacky@amd.com>
On Fri, Aug 13, 2021 at 11:59:25AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 8e7b517ad738..66ff788b79c9 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -167,7 +167,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
> }
> pte = pte_offset_kernel(pmd, vaddr);
>
> - if (sev_active())
> + if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT))
> prot = PAGE_KERNEL_EXEC;
>
> set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
> @@ -207,7 +207,7 @@ static int init_pgtable(struct kimage *image, unsigned long start_pgtable)
> level4p = (pgd_t *)__va(start_pgtable);
> clear_page(level4p);
>
> - if (sev_active()) {
> + if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT)) {
> info.page_flag |= _PAGE_ENC;
> info.kernpg_flag |= _PAGE_ENC;
> }
> @@ -570,12 +570,12 @@ void arch_kexec_unprotect_crashkres(void)
> */
> int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
> {
> - if (sev_active())
> + if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
> return 0;
>
> /*
> - * If SME is active we need to be sure that kexec pages are
> - * not encrypted because when we boot to the new kernel the
> + * If host memory encryption is active we need to be sure that kexec
> + * pages are not encrypted because when we boot to the new kernel the
> * pages won't be accessed encrypted (initially).
> */
That hunk belongs logically into the previous patch which removes
sme_active().
> return set_memory_decrypted((unsigned long)vaddr, pages);
> @@ -583,12 +583,12 @@ int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
>
> void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
> {
> - if (sev_active())
> + if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
> return;
>
> /*
> - * If SME is active we need to reset the pages back to being
> - * an encrypted mapping before freeing them.
> + * If host memory encryption is active we need to reset the pages back
> + * to being an encrypted mapping before freeing them.
> */
> set_memory_encrypted((unsigned long)vaddr, pages);
> }
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e8ccab50ebf6..b69f5ac622d5 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -25,6 +25,7 @@
> #include <linux/pagemap.h>
> #include <linux/swap.h>
> #include <linux/rwsem.h>
> +#include <linux/protected_guest.h>
>
> #include <asm/apic.h>
> #include <asm/perf_event.h>
> @@ -457,7 +458,7 @@ static int has_svm(void)
> return 0;
> }
>
> - if (sev_active()) {
> + if (prot_guest_has(PATTR_SEV)) {
> pr_info("KVM is unsupported when running as an SEV guest\n");
> return 0;
Same question as for PATTR_SME. PATTR_GUEST_MEM_ENCRYPT should be enough.
> @@ -373,7 +373,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
> * up under SME the trampoline area cannot be encrypted, whereas under SEV
> * the trampoline area must be encrypted.
> */
> -bool sev_active(void)
> +static bool sev_active(void)
> {
> return sev_status & MSR_AMD64_SEV_ENABLED;
> }
> @@ -382,7 +382,6 @@ static bool sme_active(void)
> {
> return sme_me_mask && !sev_active();
> }
> -EXPORT_SYMBOL_GPL(sev_active);
Just get rid of it altogether.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 2/3] powerpc: Fix undefined static key
From: Christophe Leroy @ 2021-08-17 9:21 UTC (permalink / raw)
To: Joel Stanley
Cc: Michael Neuling, Nicholas Piggin, Paul Mackerras, linuxppc-dev,
Daniel Axtens
In-Reply-To: <CACPK8XcVEUMS0G+NBJY4B0xE6sFFtYH5-RJtQ8J+KiuyOQx=1A@mail.gmail.com>
Le 17/08/2021 à 04:44, Joel Stanley a écrit :
> On Mon, 16 Aug 2021 at 08:39, Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 16/08/2021 à 10:24, Joel Stanley a écrit :
>>> When CONFIG_PPC_BARRIER_NOSPEC=n, security.c is not built leading to a
>>> missing definition of uaccess_flush_key.
>>>
>>> LD vmlinux.o
>>> MODPOST vmlinux.symvers
>>> MODINFO modules.builtin.modinfo
>>> GEN modules.builtin
>>> LD .tmp_vmlinux.kallsyms1
>>> powerpc64le-linux-gnu-ld: arch/powerpc/kernel/align.o:(.toc+0x0): undefined reference to `uaccess_flush_key'
>>> powerpc64le-linux-gnu-ld: arch/powerpc/kernel/signal_64.o:(.toc+0x0): undefined reference to `uaccess_flush_key'
>>> powerpc64le-linux-gnu-ld: arch/powerpc/kernel/process.o:(.toc+0x0): undefined reference to `uaccess_flush_key'
>>> powerpc64le-linux-gnu-ld: arch/powerpc/kernel/traps.o:(.toc+0x0): undefined reference to `uaccess_flush_key'
>>> powerpc64le-linux-gnu-ld: arch/powerpc/kernel/hw_breakpoint_constraints.o:(.toc+0x0): undefined reference to `uaccess_flush_key'
>>> powerpc64le-linux-gnu-ld: arch/powerpc/kernel/ptrace/ptrace.o:(.toc+0x0): more undefined references to `uaccess_flush_key' follow
>>> make[1]: *** [Makefile:1176: vmlinux] Error 1
>>>
>>> Hack one in to fix the build.
>>>
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> ---
>>> arch/powerpc/include/asm/security_features.h | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/security_features.h b/arch/powerpc/include/asm/security_features.h
>>> index 792eefaf230b..46ade7927a4c 100644
>>> --- a/arch/powerpc/include/asm/security_features.h
>>> +++ b/arch/powerpc/include/asm/security_features.h
>>> @@ -39,6 +39,9 @@ static inline bool security_ftr_enabled(u64 feature)
>>> return !!(powerpc_security_features & feature);
>>> }
>>>
>>> +#ifndef CONFIG_PPC_BARRIER_NOSPEC
>>> +DEFINE_STATIC_KEY_FALSE(uaccess_flush_key);
>>> +#endif
>>
>> It will then be re-defined by each file that includes asm/security_features.h ....
>>
>> You can't use a DEFINE_ in a .h
>>
>> You have to fix the problem at its source.
>>
>> Cleanest way I see it to modify asm/book3s/64/kup.h and something like
>>
>> if (IS_ENABLED(CONFIG_PPC_BARRIER_NOSPEC) && static_branch_unlikely(&uaccess_flush_key)
>
> This won't work either as there's no declaration for uaccess_flush_key:
>
> arch/powerpc/include/asm/book3s/64/kup.h:411:78: error:
> ‘uaccess_flush_key’ undeclared (first use in this function)
I can't understand that.
According to your commit message, you have reached linking step. It means that the necessary
declarations were present.
Adding a condition on IS_ENABLED(CONFIG_PPC_BARRIER_NOSPEC) will get you rid of the code past that
condition at compile step, then linking will succeed because the code referencing
'uaccess_flush_key' won't be there anymore when you proceed with linking.
>
> We could add an extern for it, but that is distasteful as the static
> key API suggests using the structs directly is deprecated, and the
> macros we're supposed to use perform initialisation.
You already have DECLARE_STATIC_KEY_FALSE(uaccess_flush_key); in <asm/book3s/64/kup.h>
That doesn't initialise anythink but provides the declaration.
>
> Could we assume microwatt-like platforms will not gain pkeys support,
> and have a stubbed out set of prevent/restore_user_access for systems
> that turn off either pkeys or BARRIER_NOSPEC?
>
> Or do we get rid of PPC_BARRIER_NOSPEC as an option, and have machines
> rely on disabling it at runtime?
>
^ permalink raw reply
* Re: [PATCH v2 05/12] x86/sme: Replace occurrences of sme_active() with prot_guest_has()
From: Borislav Petkov @ 2021-08-17 9:00 UTC (permalink / raw)
To: Tom Lendacky
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
Will Deacon, linux-s390, Andi Kleen, Joerg Roedel, x86, amd-gfx,
Ingo Molnar, linux-graphics-maintainer, Joerg Roedel, Tianyu Lan,
Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel, iommu,
linux-fsdevel, linuxppc-dev
In-Reply-To: <c6c38d6253dc78381f9ff0f1823b6ee5ddeefacc.1628873970.git.thomas.lendacky@amd.com>
On Fri, Aug 13, 2021 at 11:59:24AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index edc67ddf065d..5635ca9a1fbe 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data)
> struct boot_params *boot_data;
> unsigned long cmdline_paddr;
>
> - if (!sme_active())
> + if (!amd_prot_guest_has(PATTR_SME))
> return;
>
> /* Get the command line address before unmapping the real_mode_data */
> @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data)
> struct boot_params *boot_data;
> unsigned long cmdline_paddr;
>
> - if (!sme_active())
> + if (!amd_prot_guest_has(PATTR_SME))
> return;
>
> __sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true);
> @@ -378,7 +378,7 @@ bool sev_active(void)
> return sev_status & MSR_AMD64_SEV_ENABLED;
> }
>
> -bool sme_active(void)
> +static bool sme_active(void)
Just get rid of it altogether. Also, there's an
EXPORT_SYMBOL_GPL(sev_active);
which needs to go under the actual function. Here's a diff ontop:
---
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 5635ca9a1fbe..a3a2396362a5 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -364,8 +364,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
/*
* SME and SEV are very similar but they are not the same, so there are
* times that the kernel will need to distinguish between SME and SEV. The
- * sme_active() and sev_active() functions are used for this. When a
- * distinction isn't needed, the mem_encrypt_active() function can be used.
+ * PATTR_HOST_MEM_ENCRYPT and PATTR_GUEST_MEM_ENCRYPT flags to
+ * amd_prot_guest_has() are used for this. When a distinction isn't needed,
+ * the mem_encrypt_active() function can be used.
*
* The trampoline code is a good example for this requirement. Before
* paging is activated, SME will access all memory as decrypted, but SEV
@@ -377,11 +378,6 @@ bool sev_active(void)
{
return sev_status & MSR_AMD64_SEV_ENABLED;
}
-
-static bool sme_active(void)
-{
- return sme_me_mask && !sev_active();
-}
EXPORT_SYMBOL_GPL(sev_active);
/* Needs to be called from non-instrumentable code */
@@ -398,7 +394,7 @@ bool amd_prot_guest_has(unsigned int attr)
case PATTR_SME:
case PATTR_HOST_MEM_ENCRYPT:
- return sme_active();
+ return sme_me_mask && !sev_active();
case PATTR_SEV:
case PATTR_GUEST_MEM_ENCRYPT:
> {
> return sme_me_mask && !sev_active();
> }
> @@ -428,7 +428,7 @@ bool force_dma_unencrypted(struct device *dev)
> * device does not support DMA to addresses that include the
> * encryption mask.
> */
> - if (sme_active()) {
> + if (amd_prot_guest_has(PATTR_SME)) {
So I'm not sure: you add PATTR_SME which you call with
amd_prot_guest_has() and PATTR_HOST_MEM_ENCRYPT which you call with
prot_guest_has() and they both end up being the same thing on AMD.
So why even bother with PATTR_SME?
This is only going to cause confusion later and I'd say let's simply use
prot_guest_has(PATTR_HOST_MEM_ENCRYPT) everywhere...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related
* Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
From: Borislav Petkov @ 2021-08-17 8:35 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
dri-devel, linux-fsdevel, Tianyu Lan, linuxppc-dev,
Paul Mackerras
In-Reply-To: <000f627ce20c6504dd8d118d85bd69e7717b752f.1628873970.git.thomas.lendacky@amd.com>
On Fri, Aug 13, 2021 at 11:59:23AM -0500, Tom Lendacky wrote:
> Introduce a powerpc version of the prot_guest_has() function. This will
> be used to replace the powerpc mem_encrypt_active() implementation, so
> the implementation will initially only support the PATTR_MEM_ENCRYPT
> attribute.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/powerpc/include/asm/protected_guest.h | 30 ++++++++++++++++++++++
> arch/powerpc/platforms/pseries/Kconfig | 1 +
> 2 files changed, 31 insertions(+)
> create mode 100644 arch/powerpc/include/asm/protected_guest.h
>
> diff --git a/arch/powerpc/include/asm/protected_guest.h b/arch/powerpc/include/asm/protected_guest.h
> new file mode 100644
> index 000000000000..ce55c2c7e534
> --- /dev/null
> +++ b/arch/powerpc/include/asm/protected_guest.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Protected Guest (and Host) Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky <thomas.lendacky@amd.com>
> + */
> +
> +#ifndef _POWERPC_PROTECTED_GUEST_H
> +#define _POWERPC_PROTECTED_GUEST_H
> +
> +#include <asm/svm.h>
> +
> +#ifndef __ASSEMBLY__
Same thing here. Pls audit the whole set whether those __ASSEMBLY__
guards are really needed and remove them if not.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: kajoljain @ 2021-08-17 8:29 UTC (permalink / raw)
To: Madhavan Srinivasan, Christophe Leroy, Michael Ellerman,
linuxppc-dev
Cc: atrajeev, Sukadev Bhattiprolu, rnsastry
In-Reply-To: <a6217bc6-e69d-0f8a-5e85-1d776175caf7@linux.ibm.com>
On 8/17/21 11:07 AM, Madhavan Srinivasan wrote:
>
> On 8/16/21 12:26 PM, Christophe Leroy wrote:
>>
>>
>> Le 16/08/2021 à 08:44, kajoljain a écrit :
>>>
>>>
>>> On 8/14/21 6:14 PM, Michael Ellerman wrote:
>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>> Le 13/08/2021 à 10:24, Kajol Jain a écrit :
>>>>>> Incase of random sampling, there can be scenarios where SIAR is not
>>>>>> latching sample address and results in 0 value. Since current code
>>>>>> directly returning the siar value, we could see multiple instruction
>>>>>> pointer values as 0 in perf report.
>>>>
>>>> Can you please give more detail on that? What scenarios? On what CPUs?
>>>>
>>>
>>> Hi Michael,
>>> Sure I will update these details in my next patch-set.
>>>
>>>>>> Patch resolves this issue by adding a ternary condition to return
>>>>>> regs->nip incase SIAR is 0.
>>>>>
>>>>> Your description seems rather similar to
>>>>> https://github.com/linuxppc/linux/commit/2ca13a4cc56c920a6c9fc8ee45d02bccacd7f46c
>>>>>
>>>>> Does it mean that the problem occurs on more than the power10 DD1 ?
>>>>>
>>>>> In that case, can the solution be common instead of doing something for power10 DD1 and something
>>>>> for others ?
>>>>
>>>> Agreed.
>>>>
>>>> This change would seem to make that P10 DD1 logic superfluous.
>>>>
>>>> Also we already have a fallback to regs->nip in the else case of the if,
>>>> so we should just use that rather than adding a ternary condition.
>>>>
>>>> eg.
>>>>
>>>> if (use_siar && siar_valid(regs) && siar)
>>>> return siar + perf_ip_adjust(regs);
>>>> else if (use_siar)
>>>> return 0; // no valid instruction pointer
>>>> else
>>>> return regs->nip;
>>>>
>>>>
>>>> I'm also not sure why we have that return 0 case, I can't think of why
>>>> we'd ever want to do that rather than using nip. So maybe we should do
>>>> another patch to drop that case.
>>>
>>> Yeah make sense. I will remove return 0 case in my next version.
>>>
>>
>> This was added by commit https://github.com/linuxppc/linux/commit/e6878835ac4794f25385522d29c634b7bbb7cca9
>>
>> Are we sure it was an error to add it and it can be removed ?
>
> pc having 0 is wrong (kernel does not execute at 0x0 or userspace).
> yeah we should drop it.
Hi Madhavan,
Sure I will add another patch to drop return 0 condition.
Thanks,
Kajol jain
>
> Maddy
>>
>> Christophe
^ permalink raw reply
* [PATCH v6 11/11] powerpc/pseries/iommu: Rename "direct window" to "dma window"
From: Leonardo Bras @ 2021-08-17 6:39 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Frederic Barrat,
David Gibson, Nicolin Chen, kernel test robot
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210817063929.38701-1-leobras.c@gmail.com>
A previous change introduced the usage of DDW as a bigger indirect DMA
mapping when the DDW available size does not map the whole partition.
As most of the code that manipulates direct mappings was reused for
indirect mappings, it's necessary to rename all names and debug/info
messages to reflect that it can be used for both kinds of mapping.
This should cause no behavioural change, just adjust naming.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
arch/powerpc/platforms/pseries/iommu.c | 87 +++++++++++++-------------
1 file changed, 45 insertions(+), 42 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 0eccc29f5573..dab5c56ffd0e 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -349,7 +349,7 @@ struct dynamic_dma_window_prop {
__be32 window_shift; /* ilog2(tce_window_size) */
};
-struct direct_window {
+struct dma_win {
struct device_node *device;
const struct dynamic_dma_window_prop *prop;
struct list_head list;
@@ -369,11 +369,11 @@ struct ddw_create_response {
u32 addr_lo;
};
-static LIST_HEAD(direct_window_list);
+static LIST_HEAD(dma_win_list);
/* prevents races between memory on/offline and window creation */
-static DEFINE_SPINLOCK(direct_window_list_lock);
+static DEFINE_SPINLOCK(dma_win_list_lock);
/* protects initializing window twice for same device */
-static DEFINE_MUTEX(direct_window_init_mutex);
+static DEFINE_MUTEX(dma_win_init_mutex);
#define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
@@ -713,7 +713,10 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
pr_debug("pci_dma_bus_setup_pSeriesLP: setting up bus %pOF\n",
dn);
- /* Find nearest ibm,dma-window, walking up the device tree */
+ /*
+ * Find nearest ibm,dma-window (default DMA window), walking up the
+ * device tree
+ */
for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
if (dma_window != NULL)
@@ -869,37 +872,37 @@ static int remove_ddw(struct device_node *np, bool remove_prop, const char *win_
ret = of_remove_property(np, win);
if (ret)
- pr_warn("%pOF: failed to remove direct window property: %d\n",
+ pr_warn("%pOF: failed to remove DMA window property: %d\n",
np, ret);
return 0;
}
static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, int *window_shift)
{
- struct direct_window *window;
- const struct dynamic_dma_window_prop *direct64;
+ struct dma_win *window;
+ const struct dynamic_dma_window_prop *dma64;
bool found = false;
- spin_lock(&direct_window_list_lock);
+ spin_lock(&dma_win_list_lock);
/* check if we already created a window and dupe that config if so */
- list_for_each_entry(window, &direct_window_list, list) {
+ list_for_each_entry(window, &dma_win_list, list) {
if (window->device == pdn) {
- direct64 = window->prop;
- *dma_addr = be64_to_cpu(direct64->dma_base);
- *window_shift = be32_to_cpu(direct64->window_shift);
+ dma64 = window->prop;
+ *dma_addr = be64_to_cpu(dma64->dma_base);
+ *window_shift = be32_to_cpu(dma64->window_shift);
found = true;
break;
}
}
- spin_unlock(&direct_window_list_lock);
+ spin_unlock(&dma_win_list_lock);
return found;
}
-static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
- const struct dynamic_dma_window_prop *dma64)
+static struct dma_win *ddw_list_new_entry(struct device_node *pdn,
+ const struct dynamic_dma_window_prop *dma64)
{
- struct direct_window *window;
+ struct dma_win *window;
window = kzalloc(sizeof(*window), GFP_KERNEL);
if (!window)
@@ -915,7 +918,7 @@ static void find_existing_ddw_windows_named(const char *name)
{
int len;
struct device_node *pdn;
- struct direct_window *window;
+ struct dma_win *window;
const struct dynamic_dma_window_prop *dma64;
for_each_node_with_property(pdn, name) {
@@ -929,9 +932,9 @@ static void find_existing_ddw_windows_named(const char *name)
if (!window)
break;
- spin_lock(&direct_window_list_lock);
- list_add(&window->list, &direct_window_list);
- spin_unlock(&direct_window_list_lock);
+ spin_lock(&dma_win_list_lock);
+ list_add(&window->list, &dma_win_list);
+ spin_unlock(&dma_win_list_lock);
}
}
@@ -1231,7 +1234,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
const char *win_name;
struct device_node *dn;
u32 ddw_avail[DDW_APPLICABLE_SIZE];
- struct direct_window *window;
+ struct dma_win *window;
struct property *win64;
bool ddw_enabled = false;
struct failed_ddw_pdn *fpdn;
@@ -1244,7 +1247,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
pmem_present = dn != NULL;
of_node_put(dn);
- mutex_lock(&direct_window_init_mutex);
+ mutex_lock(&dma_win_init_mutex);
if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
direct_mapping = (len >= max_ram_len);
@@ -1324,8 +1327,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
page_shift = iommu_get_page_shift(query.page_size);
if (!page_shift) {
- dev_dbg(&dev->dev, "no supported direct page size in mask %x",
- query.page_size);
+ dev_dbg(&dev->dev, "no supported page size in mask %x",
+ query.page_size);
goto out_failed;
}
@@ -1384,7 +1387,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
ret = of_add_property(pdn, win64);
if (ret) {
- dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
+ dev_err(&dev->dev, "unable to add DMA window property for %pOF: %d",
pdn, ret);
goto out_free_prop;
}
@@ -1398,7 +1401,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
win64->value, tce_setrange_multi_pSeriesLP_walk);
if (ret) {
- dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
+ dev_info(&dev->dev, "failed to map DMA window for %pOF: %d\n",
dn, ret);
/* Make sure to clean DDW if any TCE was set*/
@@ -1443,9 +1446,9 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
set_iommu_table_base(&dev->dev, newtbl);
}
- spin_lock(&direct_window_list_lock);
- list_add(&window->list, &direct_window_list);
- spin_unlock(&direct_window_list_lock);
+ spin_lock(&dma_win_list_lock);
+ list_add(&window->list, &dma_win_list);
+ spin_unlock(&dma_win_list_lock);
dev->dev.archdata.dma_offset = win_addr;
ddw_enabled = true;
@@ -1477,7 +1480,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
list_add(&fpdn->list, &failed_ddw_pdn_list);
out_unlock:
- mutex_unlock(&direct_window_init_mutex);
+ mutex_unlock(&dma_win_init_mutex);
/*
* If we have persistent memory and the window size is only as big
@@ -1575,29 +1578,29 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action,
void *data)
{
- struct direct_window *window;
+ struct dma_win *window;
struct memory_notify *arg = data;
int ret = 0;
switch (action) {
case MEM_GOING_ONLINE:
- spin_lock(&direct_window_list_lock);
- list_for_each_entry(window, &direct_window_list, list) {
+ spin_lock(&dma_win_list_lock);
+ list_for_each_entry(window, &dma_win_list, list) {
ret |= tce_setrange_multi_pSeriesLP(arg->start_pfn,
arg->nr_pages, window->prop);
/* XXX log error */
}
- spin_unlock(&direct_window_list_lock);
+ spin_unlock(&dma_win_list_lock);
break;
case MEM_CANCEL_ONLINE:
case MEM_OFFLINE:
- spin_lock(&direct_window_list_lock);
- list_for_each_entry(window, &direct_window_list, list) {
+ spin_lock(&dma_win_list_lock);
+ list_for_each_entry(window, &dma_win_list, list) {
ret |= tce_clearrange_multi_pSeriesLP(arg->start_pfn,
arg->nr_pages, window->prop);
/* XXX log error */
}
- spin_unlock(&direct_window_list_lock);
+ spin_unlock(&dma_win_list_lock);
break;
default:
break;
@@ -1618,7 +1621,7 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
struct of_reconfig_data *rd = data;
struct device_node *np = rd->dn;
struct pci_dn *pci = PCI_DN(np);
- struct direct_window *window;
+ struct dma_win *window;
switch (action) {
case OF_RECONFIG_DETACH_NODE:
@@ -1636,15 +1639,15 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
iommu_pseries_free_group(pci->table_group,
np->full_name);
- spin_lock(&direct_window_list_lock);
- list_for_each_entry(window, &direct_window_list, list) {
+ spin_lock(&dma_win_list_lock);
+ list_for_each_entry(window, &dma_win_list, list) {
if (window->device == np) {
list_del(&window->list);
kfree(window);
break;
}
}
- spin_unlock(&direct_window_list_lock);
+ spin_unlock(&dma_win_list_lock);
break;
default:
err = NOTIFY_DONE;
--
2.32.0
^ permalink raw reply related
* [PATCH v6 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Leonardo Bras @ 2021-08-17 6:39 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Frederic Barrat,
David Gibson, Nicolin Chen, kernel test robot
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210817063929.38701-1-leobras.c@gmail.com>
So far it's assumed possible to map the guest RAM 1:1 to the bus, which
works with a small number of devices. SRIOV changes it as the user can
configure hundreds VFs and since phyp preallocates TCEs and does not
allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
per a PE to limit waste of physical pages.
As of today, if the assumed direct mapping is not possible, DDW creation
is skipped and the default DMA window "ibm,dma-window" is used instead.
By using DDW, indirect mapping can get more TCEs than available for the
default DMA window, and also get access to using much larger pagesizes
(16MB as implemented in qemu vs 4k from default DMA window), causing a
significant increase on the maximum amount of memory that can be IOMMU
mapped at the same time.
Indirect mapping will only be used if direct mapping is not a
possibility.
For indirect mapping, it's necessary to re-create the iommu_table with
the new DMA window parameters, so iommu_alloc() can use it.
Removing the default DMA window for using DDW with indirect mapping
is only allowed if there is no current IOMMU memory allocated in
the iommu_table. enable_ddw() is aborted otherwise.
Even though there won't be both direct and indirect mappings at the
same time, we can't reuse the DIRECT64_PROPNAME property name, or else
an older kexec()ed kernel can assume direct mapping, and skip
iommu_alloc(), causing undesirable behavior.
So a new property name DMA64_PROPNAME "linux,dma64-ddr-window-info"
was created to represent a DDW that does not allow direct mapping.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/platforms/pseries/iommu.c | 89 +++++++++++++++++++++-----
1 file changed, 74 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index e11c00b2dc1e..0eccc29f5573 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -375,6 +375,7 @@ static DEFINE_SPINLOCK(direct_window_list_lock);
/* protects initializing window twice for same device */
static DEFINE_MUTEX(direct_window_init_mutex);
#define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
+#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
unsigned long num_pfn, const void *arg)
@@ -940,6 +941,7 @@ static int find_existing_ddw_windows(void)
return 0;
find_existing_ddw_windows_named(DIRECT64_PROPNAME);
+ find_existing_ddw_windows_named(DMA64_PROPNAME);
return 0;
}
@@ -1226,14 +1228,17 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
struct ddw_create_response create;
int page_shift;
u64 win_addr;
+ const char *win_name;
struct device_node *dn;
u32 ddw_avail[DDW_APPLICABLE_SIZE];
struct direct_window *window;
struct property *win64;
bool ddw_enabled = false;
struct failed_ddw_pdn *fpdn;
- bool default_win_removed = false;
+ bool default_win_removed = false, direct_mapping = false;
bool pmem_present;
+ struct pci_dn *pci = PCI_DN(pdn);
+ struct iommu_table *tbl = pci->table_group->tables[0];
dn = of_find_node_by_type(NULL, "ibm,pmemory");
pmem_present = dn != NULL;
@@ -1242,6 +1247,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
mutex_lock(&direct_window_init_mutex);
if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
+ direct_mapping = (len >= max_ram_len);
ddw_enabled = true;
goto out_unlock;
}
@@ -1322,8 +1328,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
query.page_size);
goto out_failed;
}
- /* verify the window * number of ptes will map the partition */
- /* check largest block * page size > max memory hotplug addr */
+
+
/*
* The "ibm,pmemory" can appear anywhere in the address space.
* Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS
@@ -1339,13 +1345,25 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
dev_info(&dev->dev, "Skipping ibm,pmemory");
}
+ /* check if the available block * number of ptes will map everything */
if (query.largest_available_block < (1ULL << (len - page_shift))) {
dev_dbg(&dev->dev,
"can't map partition max 0x%llx with %llu %llu-sized pages\n",
1ULL << len,
query.largest_available_block,
1ULL << page_shift);
- goto out_failed;
+
+ /* DDW + IOMMU on single window may fail if there is any allocation */
+ if (default_win_removed && iommu_table_in_use(tbl)) {
+ dev_dbg(&dev->dev, "current IOMMU table in use, can't be replaced.\n");
+ goto out_failed;
+ }
+
+ len = order_base_2(query.largest_available_block << page_shift);
+ win_name = DMA64_PROPNAME;
+ } else {
+ direct_mapping = true;
+ win_name = DIRECT64_PROPNAME;
}
ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
@@ -1356,8 +1374,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
create.liobn, dn);
win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
- win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn, win_addr,
- page_shift, len);
+ win64 = ddw_property_create(win_name, create.liobn, win_addr, page_shift, len);
+
if (!win64) {
dev_info(&dev->dev,
"couldn't allocate property, property name, or value\n");
@@ -1375,15 +1393,54 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
if (!window)
goto out_del_prop;
- ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
- win64->value, tce_setrange_multi_pSeriesLP_walk);
- if (ret) {
- dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
- dn, ret);
+ if (direct_mapping) {
+ /* DDW maps the whole partition, so enable direct DMA mapping */
+ ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
+ win64->value, tce_setrange_multi_pSeriesLP_walk);
+ if (ret) {
+ dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
+ dn, ret);
/* Make sure to clean DDW if any TCE was set*/
clean_dma_window(pdn, win64->value);
- goto out_del_list;
+ goto out_del_list;
+ }
+ } else {
+ struct iommu_table *newtbl;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pci->phb->mem_resources); i++) {
+ const unsigned long mask = IORESOURCE_MEM_64 | IORESOURCE_MEM;
+
+ /* Look for MMIO32 */
+ if ((pci->phb->mem_resources[i].flags & mask) == IORESOURCE_MEM)
+ break;
+ }
+
+ if (i == ARRAY_SIZE(pci->phb->mem_resources))
+ goto out_del_list;
+
+ /* New table for using DDW instead of the default DMA window */
+ newtbl = iommu_pseries_alloc_table(pci->phb->node);
+ if (!newtbl) {
+ dev_dbg(&dev->dev, "couldn't create new IOMMU table\n");
+ goto out_del_list;
+ }
+
+ iommu_table_setparms_common(newtbl, pci->phb->bus->number, create.liobn, win_addr,
+ 1UL << len, page_shift, NULL, &iommu_table_lpar_multi_ops);
+ iommu_init_table(newtbl, pci->phb->node, pci->phb->mem_resources[i].start,
+ pci->phb->mem_resources[i].end);
+
+ pci->table_group->tables[1] = newtbl;
+
+ /* Keep default DMA window stuct if removed */
+ if (default_win_removed) {
+ tbl->it_size = 0;
+ kfree(tbl->it_map);
+ }
+
+ set_iommu_table_base(&dev->dev, newtbl);
}
spin_lock(&direct_window_list_lock);
@@ -1427,10 +1484,10 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
* as RAM, then we failed to create a window to cover persistent
* memory and need to set the DMA limit.
*/
- if (pmem_present && ddw_enabled && (len == max_ram_len))
+ if (pmem_present && ddw_enabled && direct_mapping && len == max_ram_len)
dev->dev.bus_dma_limit = dev->dev.archdata.dma_offset + (1ULL << len);
- return ddw_enabled;
+ return ddw_enabled && direct_mapping;
}
static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
@@ -1572,7 +1629,9 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
* we have to remove the property when releasing
* the device node.
*/
- remove_ddw(np, false, DIRECT64_PROPNAME);
+ if (remove_ddw(np, false, DIRECT64_PROPNAME))
+ remove_ddw(np, false, DMA64_PROPNAME);
+
if (pci && pci->table_group)
iommu_pseries_free_group(pci->table_group,
np->full_name);
--
2.32.0
^ permalink raw reply related
* [PATCH v6 09/11] powerpc/pseries/iommu: Find existing DDW with given property name
From: Leonardo Bras @ 2021-08-17 6:39 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Frederic Barrat,
David Gibson, Nicolin Chen, kernel test robot
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210817063929.38701-1-leobras.c@gmail.com>
At the moment pseries stores information about created directly mapped
DDW window in DIRECT64_PROPNAME.
With the objective of implementing indirect DMA mapping with DDW, it's
necessary to have another propriety name to make sure kexec'ing into older
kernels does not break, as it would if we reuse DIRECT64_PROPNAME.
In order to have this, find_existing_ddw_windows() needs to be able to
look for different property names.
Extract find_existing_ddw_windows() into find_existing_ddw_windows_named()
and calls it with current property name.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
arch/powerpc/platforms/pseries/iommu.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 901f290999d0..e11c00b2dc1e 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -910,24 +910,21 @@ static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
return window;
}
-static int find_existing_ddw_windows(void)
+static void find_existing_ddw_windows_named(const char *name)
{
int len;
struct device_node *pdn;
struct direct_window *window;
- const struct dynamic_dma_window_prop *direct64;
-
- if (!firmware_has_feature(FW_FEATURE_LPAR))
- return 0;
+ const struct dynamic_dma_window_prop *dma64;
- for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
- direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
- if (!direct64 || len < sizeof(*direct64)) {
- remove_ddw(pdn, true, DIRECT64_PROPNAME);
+ for_each_node_with_property(pdn, name) {
+ dma64 = of_get_property(pdn, name, &len);
+ if (!dma64 || len < sizeof(*dma64)) {
+ remove_ddw(pdn, true, name);
continue;
}
- window = ddw_list_new_entry(pdn, direct64);
+ window = ddw_list_new_entry(pdn, dma64);
if (!window)
break;
@@ -935,6 +932,14 @@ static int find_existing_ddw_windows(void)
list_add(&window->list, &direct_window_list);
spin_unlock(&direct_window_list_lock);
}
+}
+
+static int find_existing_ddw_windows(void)
+{
+ if (!firmware_has_feature(FW_FEATURE_LPAR))
+ return 0;
+
+ find_existing_ddw_windows_named(DIRECT64_PROPNAME);
return 0;
}
--
2.32.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox