RE: [PATCH] drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware
Reviewed-by: Evan Quan
> -Original Message-
> From: amd-gfx On Behalf Of Alex
> Deucher
> Sent: Tuesday, June 11, 2019 10:51 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander
> Subject: [PATCH] drm/amdgpu: return 0 by default in
> amdgpu_pm_load_smu_firmware
>
> Fixes SI cards running on amdgpu.
>
> Fixes: 1929059893022 ("drm/amd/amdgpu: add RLC firmware to support
> raven1 refresh")
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=110883
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index d8828a37e43a..653b2491fec8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -2700,7 +2700,7 @@ void amdgpu_pm_print_power_states(struct
> amdgpu_device *adev)
>
> int amdgpu_pm_load_smu_firmware(struct amdgpu_device *adev,
> uint32_t *smu_version) {
> - int r = -EINVAL;
> + int r;
>
> if (amdgpu_sriov_vf(adev))
> return 0;
> @@ -2713,7 +2713,7 @@ int amdgpu_pm_load_smu_firmware(struct
> amdgpu_device *adev, uint32_t *smu_versio
> }
> *smu_version = adev->pm.fw_version;
> }
> - return r;
> + return 0;
> }
>
> int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> --
> 2.20.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 hmm 00/11] Various revisions from a locking/code review
On Thu, Jun 06, 2019 at 03:44:27PM -0300, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > For hmm.git: > > This patch series arised out of discussions with Jerome when looking at the > ODP changes, particularly informed by use after free races we have already > found and fixed in the ODP code (thanks to syzkaller) working with mmu > notifiers, and the discussion with Ralph on how to resolve the lifetime model. > > Overall this brings in a simplified locking scheme and easy to explain > lifetime model: > > If a hmm_range is valid, then the hmm is valid, if a hmm is valid then the mm > is allocated memory. > > If the mm needs to still be alive (ie to lock the mmap_sem, find a vma, etc) > then the mmget must be obtained via mmget_not_zero(). > > Locking of mm->hmm is shifted to use the mmap_sem consistently for all > read/write and unlocked accesses are removed. > > The use unlocked reads on 'hmm->dead' are also eliminated in favour of using > standard mmget() locking to prevent the mm from being released. Many of the > debugging checks of !range->hmm and !hmm->mm are dropped in favour of poison - > which is much clearer as to the lifetime intent. > > The trailing patches are just some random cleanups I noticed when reviewing > this code. > > This v2 incorporates alot of the good off list changes & feedback Jerome had, > and all the on-list comments too. However, now that we have the shared git I > have kept the one line change to nouveau_svm.c rather than the compat > funtions. > > I believe we can resolve this merge in the DRM tree now and keep the core > mm/hmm.c clean. DRM maintainers, please correct me if I'm wrong. > > It is on top of hmm.git, and I have a git tree of this series to ease testing > here: > > https://github.com/jgunthorpe/linux/tree/hmm > > There are still some open locking issues, as I think this remains unaddressed: > > https://lore.kernel.org/linux-mm/20190527195829.gb18...@mellanox.com/ > > I'm looking for some more acks, reviews and tests so this can move ahead to > hmm.git. AMD Folks, this is looking pretty good now, can you please give at least a Tested-by for the new driver code using this that I see in linux-next? Thanks, Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v16 04/16] mm: untag user pointers in do_pages_move
On 6/3/19 10:55 AM, Andrey Konovalov wrote: > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > do_pages_move() is used in the implementation of the move_pages syscall. > > Untag user pointers in this function. > > Reviewed-by: Catalin Marinas > Signed-off-by: Andrey Konovalov > --- > mm/migrate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index f2ecc2855a12..3930bb6fa656 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1617,6 +1617,7 @@ static int do_pages_move(struct mm_struct *mm, > nodemask_t task_nodes, > if (get_user(node, nodes + i)) > goto out_flush; > addr = (unsigned long)p; > + addr = untagged_addr(addr); Why not just "addr = (unsigned long)untagged_addr(p);" -- Khalid
Re: [PATCH v16 03/16] lib, arm64: untag user pointers in strn*_user
On 6/3/19 10:55 AM, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> strncpy_from_user and strnlen_user accept user addresses as arguments, and
> do not go through the same path as copy_from_user and others, so here we
> need to handle the case of tagged user addresses separately.
>
> Untag user pointers passed to these functions.
>
> Note, that this patch only temporarily untags the pointers to perform
> validity checks, but then uses them as is to perform user memory accesses.
>
> Reviewed-by: Catalin Marinas
> Signed-off-by: Andrey Konovalov
> ---
> lib/strncpy_from_user.c | 3 ++-
> lib/strnlen_user.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
Looks good.
Reviewed-by: Khalid Aziz
>
> diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
> index 023ba9f3b99f..dccb95af6003 100644
> --- a/lib/strncpy_from_user.c
> +++ b/lib/strncpy_from_user.c
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -108,7 +109,7 @@ long strncpy_from_user(char *dst, const char __user *src,
> long count)
> return 0;
>
> max_addr = user_addr_max();
> - src_addr = (unsigned long)src;
> + src_addr = (unsigned long)untagged_addr(src);
> if (likely(src_addr < max_addr)) {
> unsigned long max = max_addr - src_addr;
> long retval;
> diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
> index 7f2db3fe311f..28ff554a1be8 100644
> --- a/lib/strnlen_user.c
> +++ b/lib/strnlen_user.c
> @@ -2,6 +2,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -109,7 +110,7 @@ long strnlen_user(const char __user *str, long count)
> return 0;
>
> max_addr = user_addr_max();
> - src_addr = (unsigned long)str;
> + src_addr = (unsigned long)untagged_addr(str);
> if (likely(src_addr < max_addr)) {
> unsigned long max = max_addr - src_addr;
> long retval;
>
Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
On Sat, Jun 08, 2019 at 01:54:25AM -0700, Christoph Hellwig wrote:
> FYI, I very much disagree with the direction this is moving.
>
> struct hmm_mirror literally is a trivial duplication of the
> mmu_notifiers. All these drivers should just use the mmu_notifiers
> directly for the mirroring part instead of building a thing wrapper
> that adds nothing but helping to manage the lifetime of struct hmm,
> which shouldn't exist to start with.
Christoph: What do you think about this sketch below?
It would replace the hmm_range/mirror/etc with a different way to
build the same locking scheme using some optional helpers linked to
the mmu notifier?
(just a sketch, still needs a lot more thinking)
Jason
From 5a91d17bc3b8fcaa685abddaaae5c5aea6f82dca Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe
Date: Tue, 11 Jun 2019 16:33:33 -0300
Subject: [PATCH] RFC mm: Provide helpers to implement the common mmu_notifier
locking
Many users of mmu_notifiers require a read/write lock that is write locked
during the invalidate_range_start/end period to protect against a parallel
thread reading the page tables while another thread is invalidating them.
kvm uses a collision-retry lock built with something like a sequence
count, and many mmu_notifiers users have copied this approach with various
levels of success.
Provide a common set of helpers that build a sleepable read side lock
using a collision retry scheme. The general usage pattern is:
driver pagefault():
struct mmu_invlock_state st = MMU_INVLOCK_STATE_INIT;
again:
mmu_invlock_write_start_and_lock(>mn, )
/* read vmas and page data under mmap_sem */
/* maybe sleep */
take_lock(>lock);
if (mn_invlock_end_write_and_unlock(>mn, )) {
unlock(>lock);
goto again;
}
/* make data visible to the device */
/* does not sleep */
unlock(>lock);
The driver is responsible to provide the 'driver->lock', which is the same
lock it must hold during invalidate_range_start. By holding this lock the
sequence count is fully locked, and invalidations are prevented, so it is
safe to make the work visible to the device.
Since it is possible for this to live lock it uses the write side of the
mmap_sem to create a slow path if there are repeated collisions.
This is based off the design of the hmm_range and the RDMA ODP locking
scheme, with some additional refinements.
Signed-off-by: Jason Gunthorpe
---
include/linux/mmu_notifier.h | 83
mm/mmu_notifier.c| 71 ++
2 files changed, 154 insertions(+)
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index b6c004bd9f6ad9..0417f9452f2a09 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
struct mmu_notifier;
struct mmu_notifier_ops;
@@ -227,8 +228,90 @@ struct mmu_notifier_ops {
struct mmu_notifier {
struct hlist_node hlist;
const struct mmu_notifier_ops *ops;
+
+ /*
+* mmu_invlock is a set of helpers to allow the caller to provide a
+* read/write lock scheme where the write side of the lock is held
+* between invalidate_start -> end, and the read side can be obtained
+* on some other thread. This is a common usage pattern for mmu
+* notifier users that want to lock against changes to the mmu.
+*/
+ struct mm_struct *mm;
+ unsigned int active_invalidates;
+ seqcount_t invalidate_seq;
+ wait_queue_head_t wq;
};
+struct mmu_invlock_state
+{
+ unsigned long timeout;
+ unsigned int update_seq;
+ bool write_locked;
+};
+
+#define MMU_INVLOCK_STATE_INIT {.timeout = msecs_to_jiffies(1000)}
+
+// FIXME: needs a seqcount helper
+static inline bool is_locked_seqcount(const seqcount_t *s)
+{
+ return s->sequence & 1;
+}
+
+void mmu_invlock_write_start_and_lock(struct mmu_notifier *mn,
+ struct mmu_invlock_state *st);
+bool mmu_invlock_write_end(struct mmu_notifier *mn);
+
+/**
+ * mmu_invlock_inv_start - Call during invalidate_range_start
+ * @mn - mmu_notifier
+ * @lock - True if the supplied range is interesting and should cause the
+ * write side of the lock lock to be held.
+ *
+ * Updates the locking state as part of the invalidate_range_start callback.
+ * This must be called under a user supplied lock, and it must be called for
+ * every invalidate_range_start.
+ */
+static inline void mmu_invlock_inv_start(struct mmu_notifier *mn, bool lock)
+{
+ if (lock && !mn->active_invalidates)
+ write_seqcount_begin(>invalidate_seq);
+ mn->active_invalidates++;
+}
+
+/**
+ * mmu_invlock_inv_start - Call during invalidate_range_start
+ * @mn - mmu_notifier
+ *
+ * Updates the locking state as part of the invalidate_range_start callback.
+ * This must be called under a user supplied lock, and it must be called for
+ * every invalidate_range_end.
+
Re: [PATCH v16 16/16] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Tue, Jun 11, 2019 at 07:18:04PM +0200, Andrey Konovalov wrote:
> On Tue, Jun 11, 2019 at 5:01 PM Catalin Marinas
> wrote:
> > static void *tag_ptr(void *ptr)
> > {
> > static int tagged_addr_err = 1;
> > unsigned long tag = 0;
> >
> > if (tagged_addr_err == 1)
> > tagged_addr_err = prctl(PR_SET_TAGGED_ADDR_CTRL,
> > PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
>
> I think this requires atomics. malloc() can be called from multiple threads.
It's slightly racy but I assume in a real libc it can be initialised
earlier than the hook calls while still in single-threaded mode (I had
a quick attempt with __attribute__((constructor)) but didn't get far).
Even with the race, under normal circumstances calling the prctl() twice
is not a problem. I think the risk here is that someone disables the ABI
via sysctl and the ABI is enabled for some of the threads only.
--
Catalin
Re: [PATCH v16 05/16] arm64: untag user pointers passed to memory syscalls
On Tue, Jun 11, 2019 at 05:35:31PM +0200, Andrey Konovalov wrote: > On Mon, Jun 10, 2019 at 4:28 PM Catalin Marinas > wrote: > > On Mon, Jun 03, 2019 at 06:55:07PM +0200, Andrey Konovalov wrote: > > > This patch is a part of a series that extends arm64 kernel ABI to allow to > > > pass tagged user pointers (with the top byte set to something else other > > > than 0x00) as syscall arguments. > > > > > > This patch allows tagged pointers to be passed to the following memory > > > syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect, > > > mremap, msync, munlock. > > > > > > Signed-off-by: Andrey Konovalov > > > > I would add in the commit log (and possibly in the code with a comment) > > that mremap() and mmap() do not currently accept tagged hint addresses. > > Architectures may interpret the hint tag as a background colour for the > > corresponding vma. With this: > > I'll change the commit log. Where do you you think I should put this > comment? Before mmap and mremap definitions in mm/? On arm64 we use our own sys_mmap(). I'd say just add a comment on the generic mremap() just before the untagged_addr() along the lines that new_address is not untagged for preserving similar behaviour to mmap(). -- Catalin
Re: [PATCH v16 02/16] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
On Tue, Jun 11, 2019 at 07:09:46PM +0200, Andrey Konovalov wrote: > On Tue, Jun 11, 2019 at 4:57 PM Catalin Marinas > wrote: > > > > On Mon, Jun 10, 2019 at 06:53:27PM +0100, Catalin Marinas wrote: > > > On Mon, Jun 03, 2019 at 06:55:04PM +0200, Andrey Konovalov wrote: > > > > diff --git a/arch/arm64/include/asm/uaccess.h > > > > b/arch/arm64/include/asm/uaccess.h > > > > index e5d5f31c6d36..9164ecb5feca 100644 > > > > --- a/arch/arm64/include/asm/uaccess.h > > > > +++ b/arch/arm64/include/asm/uaccess.h > > > > @@ -94,7 +94,7 @@ static inline unsigned long __range_ok(const void > > > > __user *addr, unsigned long si > > > > return ret; > > > > } > > > > > > > > -#define access_ok(addr, size) __range_ok(addr, size) > > > > +#define access_ok(addr, size) __range_ok(untagged_addr(addr), > > > > size) > > > > > > I'm going to propose an opt-in method here (RFC for now). We can't have > > > a check in untagged_addr() since this is already used throughout the > > > kernel for both user and kernel addresses (khwasan) but we can add one > > > in __range_ok(). The same prctl() option will be used for controlling > > > the precise/imprecise mode of MTE later on. We can use a TIF_ flag here > > > assuming that this will be called early on and any cloned thread will > > > inherit this. > > > > Updated patch, inlining it below. Once we agreed on the approach, I > > think Andrey can insert in in this series, probably after patch 2. The > > differences from the one I posted yesterday: > > > > - renamed PR_* macros together with get/set variants and the possibility > > to disable the relaxed ABI > > > > - sysctl option - /proc/sys/abi/tagged_addr to disable the ABI globally > > (just the prctl() opt-in, tasks already using it won't be affected) > > > > And, of course, it needs more testing. > > Sure, I'll add it to the series. > > Should I drop access_ok() change from my patch, since yours just reverts it? Not necessary, your patch just relaxes the ABI for all apps, mine tightens it. You could instead move the untagging to __range_ok() and rebase my patch accordingly. -- Catalin
Re: [PATCH v16 16/16] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Tue, Jun 11, 2019 at 5:01 PM Catalin Marinas wrote:
>
> On Mon, Jun 03, 2019 at 06:55:18PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > pass tagged user pointers (with the top byte set to something else other
> > than 0x00) as syscall arguments.
> >
> > This patch adds a simple test, that calls the uname syscall with a
> > tagged user pointer as an argument. Without the kernel accepting tagged
> > user pointers the test fails with EFAULT.
> >
> > Signed-off-by: Andrey Konovalov
>
> BTW, you could add
>
> Co-developed-by: Catalin Marinas
>
> since I wrote the malloc() etc. hooks.
Sure!
>
>
> > +static void *tag_ptr(void *ptr)
> > +{
> > + unsigned long tag = rand() & 0xff;
> > + if (!ptr)
> > + return ptr;
> > + return (void *)((unsigned long)ptr | (tag << TAG_SHIFT));
> > +}
>
> With the prctl() option, this function becomes (if you have a better
> idea, fine by me):
>
> --8<---
> #include
> #include
>
> #define TAG_SHIFT (56)
> #define TAG_MASK(0xffUL << TAG_SHIFT)
>
> #define PR_SET_TAGGED_ADDR_CTRL 55
> #define PR_GET_TAGGED_ADDR_CTRL 56
> # define PR_TAGGED_ADDR_ENABLE (1UL << 0)
>
> void *__libc_malloc(size_t size);
> void __libc_free(void *ptr);
> void *__libc_realloc(void *ptr, size_t size);
> void *__libc_calloc(size_t nmemb, size_t size);
>
> static void *tag_ptr(void *ptr)
> {
> static int tagged_addr_err = 1;
> unsigned long tag = 0;
>
> if (tagged_addr_err == 1)
> tagged_addr_err = prctl(PR_SET_TAGGED_ADDR_CTRL,
> PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
I think this requires atomics. malloc() can be called from multiple threads.
>
> if (!ptr)
> return ptr;
> if (!tagged_addr_err)
> tag = rand() & 0xff;
>
> return (void *)((unsigned long)ptr | (tag << TAG_SHIFT));
> }
>
> --
> Catalin
Re: [PATCH v16 02/16] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
On Tue, Jun 11, 2019 at 4:57 PM Catalin Marinas wrote:
>
> On Mon, Jun 10, 2019 at 06:53:27PM +0100, Catalin Marinas wrote:
> > On Mon, Jun 03, 2019 at 06:55:04PM +0200, Andrey Konovalov wrote:
> > > diff --git a/arch/arm64/include/asm/uaccess.h
> > > b/arch/arm64/include/asm/uaccess.h
> > > index e5d5f31c6d36..9164ecb5feca 100644
> > > --- a/arch/arm64/include/asm/uaccess.h
> > > +++ b/arch/arm64/include/asm/uaccess.h
> > > @@ -94,7 +94,7 @@ static inline unsigned long __range_ok(const void
> > > __user *addr, unsigned long si
> > > return ret;
> > > }
> > >
> > > -#define access_ok(addr, size) __range_ok(addr, size)
> > > +#define access_ok(addr, size) __range_ok(untagged_addr(addr), size)
> >
> > I'm going to propose an opt-in method here (RFC for now). We can't have
> > a check in untagged_addr() since this is already used throughout the
> > kernel for both user and kernel addresses (khwasan) but we can add one
> > in __range_ok(). The same prctl() option will be used for controlling
> > the precise/imprecise mode of MTE later on. We can use a TIF_ flag here
> > assuming that this will be called early on and any cloned thread will
> > inherit this.
>
> Updated patch, inlining it below. Once we agreed on the approach, I
> think Andrey can insert in in this series, probably after patch 2. The
> differences from the one I posted yesterday:
>
> - renamed PR_* macros together with get/set variants and the possibility
> to disable the relaxed ABI
>
> - sysctl option - /proc/sys/abi/tagged_addr to disable the ABI globally
> (just the prctl() opt-in, tasks already using it won't be affected)
>
> And, of course, it needs more testing.
Sure, I'll add it to the series.
Should I drop access_ok() change from my patch, since yours just reverts it?
Thanks!
>
> -8<
> From 7c624777a4e545522dec1b34e60f0229cb2bd59f Mon Sep 17 00:00:00 2001
> From: Catalin Marinas
> Date: Tue, 11 Jun 2019 13:03:38 +0100
> Subject: [PATCH] arm64: Introduce prctl() options to control the tagged user
> addresses ABI
>
> It is not desirable to relax the ABI to allow tagged user addresses into
> the kernel indiscriminately. This patch introduces a prctl() interface
> for enabling or disabling the tagged ABI with a global sysctl control
> for preventing applications from enabling the relaxed ABI (meant for
> testing user-space prctl() return error checking without reconfiguring
> the kernel). The ABI properties are inherited by threads of the same
> application and fork()'ed children but cleared on execve().
>
> The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
> MTE-specific settings like imprecise vs precise exceptions.
>
> Signed-off-by: Catalin Marinas
> ---
> arch/arm64/include/asm/processor.h | 6 +++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/include/asm/uaccess.h | 5 ++-
> arch/arm64/kernel/process.c | 67
> include/uapi/linux/prctl.h | 5 +++
> kernel/sys.c | 16 +++
> 6 files changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/processor.h
> b/arch/arm64/include/asm/processor.h
> index fcd0e691b1ea..fee457456aa8 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -307,6 +307,12 @@ extern void __init minsigstksz_setup(void);
> /* PR_PAC_RESET_KEYS prctl */
> #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
>
> +/* PR_TAGGED_ADDR prctl */
> +long set_tagged_addr_ctrl(unsigned long arg);
> +long get_tagged_addr_ctrl(void);
> +#define SET_TAGGED_ADDR_CTRL(arg) set_tagged_addr_ctrl(arg)
> +#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl()
> +
> /*
> * For CONFIG_GCC_PLUGIN_STACKLEAK
> *
> diff --git a/arch/arm64/include/asm/thread_info.h
> b/arch/arm64/include/asm/thread_info.h
> index c285d1ce7186..7263d4c973ce 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -101,6 +101,7 @@ void arch_release_task_struct(struct task_struct *tsk);
> #define TIF_SVE23 /* Scalable Vector Extension
> in use */
> #define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec
> */
> #define TIF_SSBD 25 /* Wants SSB mitigation */
> +#define TIF_TAGGED_ADDR26
>
> #define _TIF_SIGPENDING(1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
> diff --git a/arch/arm64/include/asm/uaccess.h
> b/arch/arm64/include/asm/uaccess.h
> index 9164ecb5feca..995b9ea11a89 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -73,6 +73,9 @@ static inline unsigned long __range_ok(const void __user
> *addr, unsigned long si
> {
> unsigned long ret, limit = current_thread_info()->addr_limit;
>
> + if (test_thread_flag(TIF_TAGGED_ADDR))
> +
Re: [PATCH v16 02/16] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
Hi Catalin,
...
> -8<
> From 7c624777a4e545522dec1b34e60f0229cb2bd59f Mon Sep 17 00:00:00 2001
> From: Catalin Marinas
> Date: Tue, 11 Jun 2019 13:03:38 +0100
> Subject: [PATCH] arm64: Introduce prctl() options to control the tagged user
> addresses ABI
>
> It is not desirable to relax the ABI to allow tagged user addresses into
> the kernel indiscriminately. This patch introduces a prctl() interface
> for enabling or disabling the tagged ABI with a global sysctl control
> for preventing applications from enabling the relaxed ABI (meant for
> testing user-space prctl() return error checking without reconfiguring
> the kernel). The ABI properties are inherited by threads of the same
> application and fork()'ed children but cleared on execve().
>
> The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
> MTE-specific settings like imprecise vs precise exceptions.
>
> Signed-off-by: Catalin Marinas
> ---
> arch/arm64/include/asm/processor.h | 6 +++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/include/asm/uaccess.h | 5 ++-
> arch/arm64/kernel/process.c | 67
> include/uapi/linux/prctl.h | 5 +++
> kernel/sys.c | 16 +++
> 6 files changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/processor.h
> b/arch/arm64/include/asm/processor.h
> index fcd0e691b1ea..fee457456aa8 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -307,6 +307,12 @@ extern void __init minsigstksz_setup(void);
> /* PR_PAC_RESET_KEYS prctl */
> #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
>
> +/* PR_TAGGED_ADDR prctl */
> +long set_tagged_addr_ctrl(unsigned long arg);
> +long get_tagged_addr_ctrl(void);
> +#define SET_TAGGED_ADDR_CTRL(arg)set_tagged_addr_ctrl(arg)
> +#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl()
> +
> /*
> * For CONFIG_GCC_PLUGIN_STACKLEAK
> *
> diff --git a/arch/arm64/include/asm/thread_info.h
> b/arch/arm64/include/asm/thread_info.h
> index c285d1ce7186..7263d4c973ce 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -101,6 +101,7 @@ void arch_release_task_struct(struct task_struct *tsk);
> #define TIF_SVE 23 /* Scalable Vector Extension in
> use */
> #define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec */
> #define TIF_SSBD 25 /* Wants SSB mitigation */
> +#define TIF_TAGGED_ADDR 26
>
Can you please put a comment here?
> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED(1 << TIF_NEED_RESCHED)
> diff --git a/arch/arm64/include/asm/uaccess.h
> b/arch/arm64/include/asm/uaccess.h
> index 9164ecb5feca..995b9ea11a89 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -73,6 +73,9 @@ static inline unsigned long __range_ok(const void __user
> *addr, unsigned long si
> {
> unsigned long ret, limit = current_thread_info()->addr_limit;
>
> + if (test_thread_flag(TIF_TAGGED_ADDR))
> + addr = untagged_addr(addr);
> +
> __chk_user_ptr(addr);
> asm volatile(
> // A + B <= C + 1 for all A,B,C, in four easy steps:
> @@ -94,7 +97,7 @@ static inline unsigned long __range_ok(const void __user
> *addr, unsigned long si
> return ret;
> }
>
> -#define access_ok(addr, size)__range_ok(untagged_addr(addr), size)
> +#define access_ok(addr, size)__range_ok(addr, size)
> #define user_addr_maxget_fs
>> #define _ASM_EXTABLE(from, to)
>> \
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 3767fb21a5b8..69d0be1fc708 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -323,6 +324,7 @@ void flush_thread(void)
> fpsimd_flush_thread();
> tls_thread_flush();
> flush_ptrace_hw_breakpoint(current);
> + clear_thread_flag(TIF_TAGGED_ADDR);
Nit: in line we the other functions in thread_flush we could have something like
"tagged_addr_thread_flush", maybe inlined.
> }
>
> void release_thread(struct task_struct *dead_task)
> @@ -552,3 +554,68 @@ void arch_setup_new_exec(void)
>
> ptrauth_thread_init_user(current);
> }
> +
> +/*
> + * Control the relaxed ABI allowing tagged user addresses into the kernel.
> + */
> +static unsigned int tagged_addr_prctl_allowed = 1;
> +
> +long set_tagged_addr_ctrl(unsigned long arg)
> +{
> + if (!tagged_addr_prctl_allowed)
> + return -EINVAL;
> + if (is_compat_task())
> + return -EINVAL;
> + if (arg & ~PR_TAGGED_ADDR_ENABLE)
> + return -EINVAL;
[PATCH] drm/amd/amdgpu: Bail out of BO node creation if not enough VRAM (v2)
(v2): Return 0 and set mem->mm_node to NULL.
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8aea2f21b202..c2730e5c1081 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -276,7 +276,7 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
struct drm_mm_node *nodes;
enum drm_mm_insert_mode mode;
unsigned long lpfn, num_nodes, pages_per_node, pages_left;
- uint64_t usage = 0, vis_usage = 0;
+ uint64_t vis_usage = 0;
unsigned i;
int r;
@@ -284,6 +284,14 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
if (!lpfn)
lpfn = man->size;
+ /* bail out quickly if there's likely not enough VRAM for this BO */
+ atomic64_add(mem->num_pages << PAGE_SHIFT, >usage);
+ if (atomic64_read(>usage) > adev->gmc.mc_vram_size) {
+ atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
+ mem->mm_node = NULL;
+ return 0;
+ }
+
if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
@@ -300,8 +308,10 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
GFP_KERNEL | __GFP_ZERO);
- if (!nodes)
+ if (!nodes) {
+ atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
return -ENOMEM;
+ }
mode = DRM_MM_INSERT_BEST;
if (place->flags & TTM_PL_FLAG_TOPDOWN)
@@ -321,7 +331,6 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
if (unlikely(r))
break;
- usage += nodes[i].size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, [i]);
amdgpu_vram_mgr_virt_start(mem, [i]);
pages_left -= pages;
@@ -341,14 +350,12 @@ static int amdgpu_vram_mgr_new(struct
ttm_mem_type_manager *man,
if (unlikely(r))
goto error;
- usage += nodes[i].size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, [i]);
amdgpu_vram_mgr_virt_start(mem, [i]);
pages_left -= pages;
}
spin_unlock(>lock);
- atomic64_add(usage, >usage);
atomic64_add(vis_usage, >vis_usage);
mem->mm_node = nodes;
@@ -359,6 +366,7 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
while (i--)
drm_mm_remove_node([i]);
spin_unlock(>lock);
+ atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
kvfree(nodes);
return r == -ENOSPC ? 0 : r;
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/amdgpu: Bail out of BO node creation if not enough VRAM
On 2019-06-11 9:34 a.m., Christian König wrote:
> Am 10.06.19 um 16:32 schrieb StDenis, Tom:
>> Signed-off-by: Tom St Denis
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 17 -
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index 8aea2f21b202..70b4a5a97ed2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -276,7 +276,7 @@ static int amdgpu_vram_mgr_new(struct
>> ttm_mem_type_manager *man,
>> struct drm_mm_node *nodes;
>> enum drm_mm_insert_mode mode;
>> unsigned long lpfn, num_nodes, pages_per_node, pages_left;
>> - uint64_t usage = 0, vis_usage = 0;
>> + uint64_t vis_usage = 0;
>> unsigned i;
>> int r;
>> @@ -284,6 +284,13 @@ static int amdgpu_vram_mgr_new(struct
>> ttm_mem_type_manager *man,
>> if (!lpfn)
>> lpfn = man->size;
>> + /* bail out quickly if there's likely not enough VRAM for this
>> BO */
>> + atomic64_add(mem->num_pages << PAGE_SHIFT, >usage);
>> + if (atomic64_read(>usage) > adev->gmc.mc_vram_size) {
>> + atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
>> + return -ENOSPC;
>> + }
>
> That's not 100% correct IIRC. See TTM expects a return value of 0, but
> mem->node to be NULL when allocation fails.
Yup, the node creation returns -ENOSPC which this function then
translates to 0. I'll submit a v2 in a minute once I pass a piglit test :-)
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Reserve space for shared fence
Call reservation_object_reserve_shared to reserve space for shared fence. Otherwise it will trigger BUG_ON condition in reservation_object_add_shared_fence. Change-Id: Ib0fae16247e33ee68f95bffa723451c0cd619344 Signed-off-by: Oak Zeng --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 81e0e75..74e8695 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -2152,12 +2152,16 @@ int amdgpu_amdkfd_add_gws_to_process(void *info, void *gws, struct kgd_mem **mem * Add process eviction fence to bo so they can * evict each other. */ + ret = reservation_object_reserve_shared(gws_bo->tbo.resv, 1); + if (ret) + goto reserve_shared_fail; amdgpu_bo_fence(gws_bo, _info->eviction_fence->base, true); amdgpu_bo_unreserve(gws_bo); mutex_unlock(&(*mem)->process_info->lock); return ret; +reserve_shared_fail: bo_validation_failure: amdgpu_bo_unreserve(gws_bo); bo_reservation_failure: -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v16 05/16] arm64: untag user pointers passed to memory syscalls
On Mon, Jun 10, 2019 at 4:28 PM Catalin Marinas wrote: > > On Mon, Jun 03, 2019 at 06:55:07PM +0200, Andrey Konovalov wrote: > > This patch is a part of a series that extends arm64 kernel ABI to allow to > > pass tagged user pointers (with the top byte set to something else other > > than 0x00) as syscall arguments. > > > > This patch allows tagged pointers to be passed to the following memory > > syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect, > > mremap, msync, munlock. > > > > Signed-off-by: Andrey Konovalov > > I would add in the commit log (and possibly in the code with a comment) > that mremap() and mmap() do not currently accept tagged hint addresses. > Architectures may interpret the hint tag as a background colour for the > corresponding vma. With this: I'll change the commit log. Where do you you think I should put this comment? Before mmap and mremap definitions in mm/? Thanks! > > Reviewed-by: Catalin Marinas > > -- > Catalin
Re: [PATCH v16 16/16] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Mon, Jun 03, 2019 at 06:55:18PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> This patch adds a simple test, that calls the uname syscall with a
> tagged user pointer as an argument. Without the kernel accepting tagged
> user pointers the test fails with EFAULT.
>
> Signed-off-by: Andrey Konovalov
BTW, you could add
Co-developed-by: Catalin Marinas
since I wrote the malloc() etc. hooks.
> +static void *tag_ptr(void *ptr)
> +{
> + unsigned long tag = rand() & 0xff;
> + if (!ptr)
> + return ptr;
> + return (void *)((unsigned long)ptr | (tag << TAG_SHIFT));
> +}
With the prctl() option, this function becomes (if you have a better
idea, fine by me):
--8<---
#include
#include
#define TAG_SHIFT (56)
#define TAG_MASK(0xffUL << TAG_SHIFT)
#define PR_SET_TAGGED_ADDR_CTRL 55
#define PR_GET_TAGGED_ADDR_CTRL 56
# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
void *__libc_malloc(size_t size);
void __libc_free(void *ptr);
void *__libc_realloc(void *ptr, size_t size);
void *__libc_calloc(size_t nmemb, size_t size);
static void *tag_ptr(void *ptr)
{
static int tagged_addr_err = 1;
unsigned long tag = 0;
if (tagged_addr_err == 1)
tagged_addr_err = prctl(PR_SET_TAGGED_ADDR_CTRL,
PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
if (!ptr)
return ptr;
if (!tagged_addr_err)
tag = rand() & 0xff;
return (void *)((unsigned long)ptr | (tag << TAG_SHIFT));
}
--
Catalin
Re: [PATCH v16 02/16] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
On Mon, Jun 10, 2019 at 06:53:27PM +0100, Catalin Marinas wrote:
> On Mon, Jun 03, 2019 at 06:55:04PM +0200, Andrey Konovalov wrote:
> > diff --git a/arch/arm64/include/asm/uaccess.h
> > b/arch/arm64/include/asm/uaccess.h
> > index e5d5f31c6d36..9164ecb5feca 100644
> > --- a/arch/arm64/include/asm/uaccess.h
> > +++ b/arch/arm64/include/asm/uaccess.h
> > @@ -94,7 +94,7 @@ static inline unsigned long __range_ok(const void __user
> > *addr, unsigned long si
> > return ret;
> > }
> >
> > -#define access_ok(addr, size) __range_ok(addr, size)
> > +#define access_ok(addr, size) __range_ok(untagged_addr(addr), size)
>
> I'm going to propose an opt-in method here (RFC for now). We can't have
> a check in untagged_addr() since this is already used throughout the
> kernel for both user and kernel addresses (khwasan) but we can add one
> in __range_ok(). The same prctl() option will be used for controlling
> the precise/imprecise mode of MTE later on. We can use a TIF_ flag here
> assuming that this will be called early on and any cloned thread will
> inherit this.
Updated patch, inlining it below. Once we agreed on the approach, I
think Andrey can insert in in this series, probably after patch 2. The
differences from the one I posted yesterday:
- renamed PR_* macros together with get/set variants and the possibility
to disable the relaxed ABI
- sysctl option - /proc/sys/abi/tagged_addr to disable the ABI globally
(just the prctl() opt-in, tasks already using it won't be affected)
And, of course, it needs more testing.
-8<
>From 7c624777a4e545522dec1b34e60f0229cb2bd59f Mon Sep 17 00:00:00 2001
From: Catalin Marinas
Date: Tue, 11 Jun 2019 13:03:38 +0100
Subject: [PATCH] arm64: Introduce prctl() options to control the tagged user
addresses ABI
It is not desirable to relax the ABI to allow tagged user addresses into
the kernel indiscriminately. This patch introduces a prctl() interface
for enabling or disabling the tagged ABI with a global sysctl control
for preventing applications from enabling the relaxed ABI (meant for
testing user-space prctl() return error checking without reconfiguring
the kernel). The ABI properties are inherited by threads of the same
application and fork()'ed children but cleared on execve().
The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
MTE-specific settings like imprecise vs precise exceptions.
Signed-off-by: Catalin Marinas
---
arch/arm64/include/asm/processor.h | 6 +++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/uaccess.h | 5 ++-
arch/arm64/kernel/process.c | 67
include/uapi/linux/prctl.h | 5 +++
kernel/sys.c | 16 +++
6 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/processor.h
b/arch/arm64/include/asm/processor.h
index fcd0e691b1ea..fee457456aa8 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -307,6 +307,12 @@ extern void __init minsigstksz_setup(void);
/* PR_PAC_RESET_KEYS prctl */
#define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
+/* PR_TAGGED_ADDR prctl */
+long set_tagged_addr_ctrl(unsigned long arg);
+long get_tagged_addr_ctrl(void);
+#define SET_TAGGED_ADDR_CTRL(arg) set_tagged_addr_ctrl(arg)
+#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl()
+
/*
* For CONFIG_GCC_PLUGIN_STACKLEAK
*
diff --git a/arch/arm64/include/asm/thread_info.h
b/arch/arm64/include/asm/thread_info.h
index c285d1ce7186..7263d4c973ce 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -101,6 +101,7 @@ void arch_release_task_struct(struct task_struct *tsk);
#define TIF_SVE23 /* Scalable Vector Extension in
use */
#define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec */
#define TIF_SSBD 25 /* Wants SSB mitigation */
+#define TIF_TAGGED_ADDR26
#define _TIF_SIGPENDING(1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 9164ecb5feca..995b9ea11a89 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -73,6 +73,9 @@ static inline unsigned long __range_ok(const void __user
*addr, unsigned long si
{
unsigned long ret, limit = current_thread_info()->addr_limit;
+ if (test_thread_flag(TIF_TAGGED_ADDR))
+ addr = untagged_addr(addr);
+
__chk_user_ptr(addr);
asm volatile(
// A + B <= C + 1 for all A,B,C, in four easy steps:
@@ -94,7 +97,7 @@ static inline unsigned long __range_ok(const void __user
*addr, unsigned long si
return ret;
}
-#define access_ok(addr, size) __range_ok(untagged_addr(addr), size)
+#define access_ok(addr, size)
[PATCH] drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware
Fixes SI cards running on amdgpu.
Fixes: 1929059893022 ("drm/amd/amdgpu: add RLC firmware to support raven1
refresh")
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=110883
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index d8828a37e43a..653b2491fec8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -2700,7 +2700,7 @@ void amdgpu_pm_print_power_states(struct amdgpu_device
*adev)
int amdgpu_pm_load_smu_firmware(struct amdgpu_device *adev, uint32_t
*smu_version)
{
- int r = -EINVAL;
+ int r;
if (amdgpu_sriov_vf(adev))
return 0;
@@ -2713,7 +2713,7 @@ int amdgpu_pm_load_smu_firmware(struct amdgpu_device
*adev, uint32_t *smu_versio
}
*smu_version = adev->pm.fw_version;
}
- return r;
+ return 0;
}
int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/gfx9: reset compute ring wptr when gpu reset
On Tue, Jun 4, 2019 at 11:44 AM Yintian Tao wrote:
>
> We need reset compute ring wptr to zero when gpu reset
> in order to prevent CP hang.
>
> Signed-off-by: Yintian Tao
Acked-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 2e9cac1..25211ba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3175,6 +3175,7 @@ static int gfx_v9_0_kcq_init_queue(struct amdgpu_ring
> *ring)
>
> /* reset ring buffer */
> ring->wptr = 0;
> + amdgpu_ring_set_wptr(ring);
> amdgpu_ring_clear_ring(ring);
> } else {
> amdgpu_ring_clear_ring(ring);
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v16 08/16] fs, arm64: untag user pointers in copy_mount_options
On Sat, Jun 8, 2019 at 6:02 AM Kees Cook wrote: > > On Mon, Jun 03, 2019 at 06:55:10PM +0200, Andrey Konovalov wrote: > > This patch is a part of a series that extends arm64 kernel ABI to allow to > > pass tagged user pointers (with the top byte set to something else other > > than 0x00) as syscall arguments. > > > > In copy_mount_options a user address is being subtracted from TASK_SIZE. > > If the address is lower than TASK_SIZE, the size is calculated to not > > allow the exact_copy_from_user() call to cross TASK_SIZE boundary. > > However if the address is tagged, then the size will be calculated > > incorrectly. > > > > Untag the address before subtracting. > > > > Reviewed-by: Catalin Marinas > > Signed-off-by: Andrey Konovalov > > One thing I just noticed in the commit titles... "arm64" is in the > prefix, but these are arch-indep areas. Should the ", arm64" be left > out? > > I would expect, instead: > > fs/namespace: untag user pointers in copy_mount_options Hm, I've added the arm64 tag in all of the patches because they are related to changes in arm64 kernel ABI. I can remove it from all the patches that only touch common code if you think that it makes sense. Thanks! > > Reviewed-by: Kees Cook > > -Kees > > > --- > > fs/namespace.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/namespace.c b/fs/namespace.c > > index b26778bdc236..2e85712a19ed 100644 > > --- a/fs/namespace.c > > +++ b/fs/namespace.c > > @@ -2993,7 +2993,7 @@ void *copy_mount_options(const void __user * data) > >* the remainder of the page. > >*/ > > /* copy_from_user cannot cross TASK_SIZE ! */ > > - size = TASK_SIZE - (unsigned long)data; > > + size = TASK_SIZE - (unsigned long)untagged_addr(data); > > if (size > PAGE_SIZE) > > size = PAGE_SIZE; > > > > -- > > 2.22.0.rc1.311.g5d7573a151-goog > > > > -- > Kees Cook
Re: [PATCH] drm/amd/amdgpu: Bail out of BO node creation if not enough VRAM
Am 10.06.19 um 16:32 schrieb StDenis, Tom:
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8aea2f21b202..70b4a5a97ed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -276,7 +276,7 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
struct drm_mm_node *nodes;
enum drm_mm_insert_mode mode;
unsigned long lpfn, num_nodes, pages_per_node, pages_left;
- uint64_t usage = 0, vis_usage = 0;
+ uint64_t vis_usage = 0;
unsigned i;
int r;
@@ -284,6 +284,13 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager *man,
if (!lpfn)
lpfn = man->size;
+ /* bail out quickly if there's likely not enough VRAM for this BO */
+ atomic64_add(mem->num_pages << PAGE_SHIFT, >usage);
+ if (atomic64_read(>usage) > adev->gmc.mc_vram_size) {
+ atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
+ return -ENOSPC;
+ }
That's not 100% correct IIRC. See TTM expects a return value of 0, but
mem->node to be NULL when allocation fails.
Christian.
+
if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
@@ -300,8 +307,10 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
GFP_KERNEL | __GFP_ZERO);
- if (!nodes)
+ if (!nodes) {
+ atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
return -ENOMEM;
+ }
mode = DRM_MM_INSERT_BEST;
if (place->flags & TTM_PL_FLAG_TOPDOWN)
@@ -321,7 +330,6 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
if (unlikely(r))
break;
- usage += nodes[i].size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, [i]);
amdgpu_vram_mgr_virt_start(mem, [i]);
pages_left -= pages;
@@ -341,14 +349,12 @@ static int amdgpu_vram_mgr_new(struct
ttm_mem_type_manager *man,
if (unlikely(r))
goto error;
- usage += nodes[i].size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, [i]);
amdgpu_vram_mgr_virt_start(mem, [i]);
pages_left -= pages;
}
spin_unlock(>lock);
- atomic64_add(usage, >usage);
atomic64_add(vis_usage, >vis_usage);
mem->mm_node = nodes;
@@ -359,6 +365,7 @@ static int amdgpu_vram_mgr_new(struct ttm_mem_type_manager
*man,
while (i--)
drm_mm_remove_node([i]);
spin_unlock(>lock);
+ atomic64_sub(mem->num_pages << PAGE_SHIFT, >usage);
kvfree(nodes);
return r == -ENOSPC ? 0 : r;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add CHIP_VEGAM to amdgpu_amdkfd_device_probe
Reviewed-by: Alex Deucher From: amd-gfx on behalf of Russell, Kent Sent: Tuesday, June 11, 2019 8:44 AM To: amd-gfx@lists.freedesktop.org Cc: Russell, Kent Subject: [PATCH] drm/amdgpu: Add CHIP_VEGAM to amdgpu_amdkfd_device_probe We have the rest of the support in the kerne, but we don't actually boot KFD on the device without this change Change-Id: Ifcc5c5e058d72e33a2f3f945f269ba6d2613b7b0 Signed-off-by: Kent Russell --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c index 4af3989e4a75..b7582b9aa1cd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c @@ -78,6 +78,7 @@ void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev) case CHIP_POLARIS10: case CHIP_POLARIS11: case CHIP_POLARIS12: + case CHIP_VEGAM: kfd2kgd = amdgpu_amdkfd_gfx_8_0_get_functions(); break; case CHIP_VEGA10: -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Add CHIP_VEGAM to amdgpu_amdkfd_device_probe
We have the rest of the support in the kerne, but we don't actually boot KFD on the device without this change Change-Id: Ifcc5c5e058d72e33a2f3f945f269ba6d2613b7b0 Signed-off-by: Kent Russell --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c index 4af3989e4a75..b7582b9aa1cd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c @@ -78,6 +78,7 @@ void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev) case CHIP_POLARIS10: case CHIP_POLARIS11: case CHIP_POLARIS12: + case CHIP_VEGAM: kfd2kgd = amdgpu_amdkfd_gfx_8_0_get_functions(); break; case CHIP_VEGA10: -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v1 01/10] drm: fix build errors with drm_print.h
Hi Daniel. > > > > drm_print.h uses DRM_NAME - which is defined only in uapi/drm/drm.h. > > The error is seen when you include drm_print.h and not much other drm > > header files. > > Tiday we rely on other drm header files to pull in drm.h. > > > > Notice - the error only manifest itself when the macros are used. > > Including the drm_print.h will not do it. > > Uh, I think let's just inline DRM_NAME, that also cleans up the code. And > drop the uapi include from drm internal headers, that feels bad. In > general I think uapi headers should only be included by the .c files that > actually implement ioctls and stuff like that. > > Can you pls do that patch for drm_print.h? Yep - will do in a few days. Sam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: fix pm_load_smu_firmware for SR-IOV
> -Original Message-
> From: amd-gfx On Behalf Of
> Trigger Huang
> Sent: Tuesday, June 11, 2019 12:09 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liang, Prike ; Huang, Trigger
>
> Subject: [PATCH] drm/amdgpu: fix pm_load_smu_firmware for SR-IOV
>
> For SR-IOV VF, powerplay may not be supported, in this case, error '-EINVAL'
> should not be returned.
>
> Signed-off-by: Trigger Huang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 21b5be1..4276d63 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -2709,7 +2709,10 @@ int amdgpu_pm_load_smu_firmware(struct
> amdgpu_device *adev, uint32_t *smu_versio
> return r;
> }
> *smu_version = adev->pm.fw_version;
> + } else if (amdgpu_sriov_vf(adev)) {
> + r = 0;
Could you please add the checking at the start of this function:
if (amdgpu_sriov_vf(adev))
return 0;
The code looks better than adding a "else" here.
With that fixed, please add
Reviewed-by: Huang Rui
> }
> +
> return r;
> }
>
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v1 01/10] drm: fix build errors with drm_print.h
On Tue, Jun 11, 2019 at 11:11:07AM +0200, Sam Ravnborg wrote: > Hi Daniel. > > On Tue, Jun 11, 2019 at 10:39:11AM +0200, Daniel Vetter wrote: > > On Mon, Jun 10, 2019 at 12:07:48AM +0200, Sam Ravnborg wrote: > > > drm_print.h requires to fix build when macros are used. > > > Pull in the header file in drm_print.h so users do not have to do it. > > > > > > Signed-off-by: Sam Ravnborg > > > Cc: Maarten Lankhorst > > > Cc: Maxime Ripard > > > Cc: Sean Paul > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > --- > > > include/drm/drm_print.h | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > > > index 3a4247319e63..a5d6f2f3e430 100644 > > > --- a/include/drm/drm_print.h > > > +++ b/include/drm/drm_print.h > > > @@ -32,6 +32,8 @@ > > > #include > > > #include > > > > > > +#include > > > > What goes boom without this? Only drm/drm.h that I could find was the one > > in uapi/drm/drm.h, and that one looks very fishy to include from an > > internal helper header like print.h. > > drm_print.h uses DRM_NAME - which is defined only in uapi/drm/drm.h. > The error is seen when you include drm_print.h and not much other drm > header files. > Tiday we rely on other drm header files to pull in drm.h. > > Notice - the error only manifest itself when the macros are used. > Including the drm_print.h will not do it. Uh, I think let's just inline DRM_NAME, that also cleans up the code. And drop the uapi include from drm internal headers, that feels bad. In general I think uapi headers should only be included by the .c files that actually implement ioctls and stuff like that. Can you pls do that patch for drm_print.h? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v1 01/10] drm: fix build errors with drm_print.h
Hi Daniel. On Tue, Jun 11, 2019 at 10:39:11AM +0200, Daniel Vetter wrote: > On Mon, Jun 10, 2019 at 12:07:48AM +0200, Sam Ravnborg wrote: > > drm_print.h requires to fix build when macros are used. > > Pull in the header file in drm_print.h so users do not have to do it. > > > > Signed-off-by: Sam Ravnborg > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Daniel Vetter > > --- > > include/drm/drm_print.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > > index 3a4247319e63..a5d6f2f3e430 100644 > > --- a/include/drm/drm_print.h > > +++ b/include/drm/drm_print.h > > @@ -32,6 +32,8 @@ > > #include > > #include > > > > +#include > > What goes boom without this? Only drm/drm.h that I could find was the one > in uapi/drm/drm.h, and that one looks very fishy to include from an > internal helper header like print.h. drm_print.h uses DRM_NAME - which is defined only in uapi/drm/drm.h. The error is seen when you include drm_print.h and not much other drm header files. Tiday we rely on other drm header files to pull in drm.h. Notice - the error only manifest itself when the macros are used. Including the drm_print.h will not do it. Sam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v1 01/10] drm: fix build errors with drm_print.h
On Mon, Jun 10, 2019 at 12:07:48AM +0200, Sam Ravnborg wrote: > drm_print.h requires to fix build when macros are used. > Pull in the header file in drm_print.h so users do not have to do it. > > Signed-off-by: Sam Ravnborg > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > --- > include/drm/drm_print.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index 3a4247319e63..a5d6f2f3e430 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -32,6 +32,8 @@ > #include > #include > > +#include What goes boom without this? Only drm/drm.h that I could find was the one in uapi/drm/drm.h, and that one looks very fishy to include from an internal helper header like print.h. -Daniel > + > /** > * DOC: print > * > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
On 6/10/19 9:02 AM, Jason Gunthorpe wrote: On Fri, Jun 07, 2019 at 02:37:07PM -0700, Ralph Campbell wrote: On 6/6/19 11:44 AM, Jason Gunthorpe wrote: From: Jason Gunthorpe hmm_release() is called exactly once per hmm. ops->release() cannot accidentally trigger any action that would recurse back onto hmm->mirrors_sem. This fixes a use after-free race of the form: CPU0 CPU1 hmm_release() up_write(>mirrors_sem); hmm_mirror_unregister(mirror) down_write(>mirrors_sem); up_write(>mirrors_sem); kfree(mirror) mirror->ops->release(mirror) The only user we have today for ops->release is an empty function, so this is unambiguously safe. As a consequence of plugging this race drivers are not allowed to register/unregister mirrors from within a release op. Signed-off-by: Jason Gunthorpe I agree with the analysis above but I'm not sure that release() will always be an empty function. It might be more efficient to write back all data migrated to a device "in one pass" instead of relying on unmap_vmas() calling hmm_start_range_invalidate() per VMA. I think we have to focus on the *current* kernel - and we have two users of release, nouveau_svm.c is empty and amdgpu_mn.c does schedule_work() - so I believe we should go ahead with this simple solution to the actual race today that both of those will suffer from. If we find a need for a more complex version then it can be debated and justified with proper context... Ok? Jason OK. I guess we have enough on the plate already :-) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: fix pm_load_smu_firmware for SR-IOV
Reviewed-by: Prike Liang
Thanks,
Prike
> -Original Message-
> From: Trigger Huang
> Sent: Tuesday, June 11, 2019 12:09 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Huang, Trigger ; Liang, Prike
>
> Subject: [PATCH] drm/amdgpu: fix pm_load_smu_firmware for SR-IOV
>
> For SR-IOV VF, powerplay may not be supported, in this case, error '-EINVAL'
> should not be returned.
>
> Signed-off-by: Trigger Huang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 21b5be1..4276d63 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -2709,7 +2709,10 @@ int amdgpu_pm_load_smu_firmware(struct
> amdgpu_device *adev, uint32_t *smu_versio
> return r;
> }
> *smu_version = adev->pm.fw_version;
> + } else if (amdgpu_sriov_vf(adev)) {
> + r = 0;
> }
> +
> return r;
> }
>
> --
> 2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
