[PATCHv8.1 01/13] cec-notifier: rename variables, check kstrdup and n->conn_name
dev -> hdmi_dev conn -> conn_name Check if n->conn_name is not NULL before calling strcmp. Check the result of kstrdup, and clean up on error. Signed-off-by: Hans Verkuil --- Changes since v8: - check n->conn_name before calling strcmp to make the code more robust --- drivers/media/cec/cec-notifier.c | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c index 9598c7778871..f72b19c351dd 100644 --- a/drivers/media/cec/cec-notifier.c +++ b/drivers/media/cec/cec-notifier.c @@ -21,8 +21,8 @@ struct cec_notifier { struct mutex lock; struct list_head head; struct kref kref; - struct device *dev; - const char *conn; + struct device *hdmi_dev; + const char *conn_name; struct cec_adapter *cec_adap; void (*callback)(struct cec_adapter *adap, u16 pa); @@ -32,14 +32,16 @@ struct cec_notifier { static LIST_HEAD(cec_notifiers); static DEFINE_MUTEX(cec_notifiers_lock); -struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn) +struct cec_notifier * +cec_notifier_get_conn(struct device *hdmi_dev, const char *conn_name) { struct cec_notifier *n; mutex_lock(&cec_notifiers_lock); list_for_each_entry(n, &cec_notifiers, head) { - if (n->dev == dev && - (!conn || !strcmp(n->conn, conn))) { + if (n->hdmi_dev == hdmi_dev && + (!conn_name || +(n->conn_name && !strcmp(n->conn_name, conn_name { kref_get(&n->kref); mutex_unlock(&cec_notifiers_lock); return n; @@ -48,10 +50,17 @@ struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn) n = kzalloc(sizeof(*n), GFP_KERNEL); if (!n) goto unlock; - n->dev = dev; - if (conn) - n->conn = kstrdup(conn, GFP_KERNEL); + n->hdmi_dev = hdmi_dev; + if (conn_name) { + n->conn_name = kstrdup(conn_name, GFP_KERNEL); + if (!n->conn_name) { + kfree(n); + n = NULL; + goto unlock; + } + } n->phys_addr = CEC_PHYS_ADDR_INVALID; + mutex_init(&n->lock); kref_init(&n->kref); list_add_tail(&n->head, &cec_notifiers); @@ -67,7 +76,7 @@ static void cec_notifier_release(struct kref *kref) container_of(kref, struct cec_notifier, kref); list_del(&n->head); - kfree(n->conn); + kfree(n->conn_name); kfree(n); } -- 2.20.1
[PATCH v4 0/2] Add DSI panel driver for Raydium RM67191
This patch-set contains the DRM panel driver and dt-bindings documentation for the DSI driven panel: Raydium RM67191. v4: - Changed default_timing structure type from 'struct display_timing' to 'struct drm_display_mode' (fabio) - Replaced devm_gpiod_get with devm_gpiod_get_optional (fabio) - Added power regulators (fabio) - Removed pm_ops (fabio) v3: - Added myself to MAINTAINERS for this driver (sam) - Removed display-timings property (fabio) - Fixed dt description (sam) - Re-arranged calls inside get_modes function (sam) - Changed ifdefs with _maybe_unused for suspend/resume functions (sam) - Collected Reviewed-by from Sam v2: - Fixed 'reset-gpio' to 'reset-gpios' property naming (fabio) - Changed the state of the reset gpio to active low and fixed how it is handled in driver (fabio) - Fixed copyright statement (daniel) - Reordered includes (sam) - Added defines for panel specific color formats (fabio) - Removed unnecessary tests in enable and unprepare (sam) - Removed the unnecessary backlight write in enable (sam) Robert Chiras (2): dt-bindings: display: panel: Add support for Raydium RM67191 panel drm/panel: Add support for Raydium RM67191 panel driver .../bindings/display/panel/raydium,rm67191.txt | 41 ++ MAINTAINERS| 6 + drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm67191.c | 673 + 5 files changed, 730 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c -- 2.7.4
[PATCH v4 2/2] drm/panel: Add support for Raydium RM67191 panel driver
This patch adds Raydium RM67191 TFT LCD panel driver (MIPI-DSI protocol). Signed-off-by: Robert Chiras Reviewed-by: Sam Ravnborg --- MAINTAINERS | 6 + drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm67191.c | 673 ++ 4 files changed, 689 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c diff --git a/MAINTAINERS b/MAINTAINERS index 7a2f487..cd93030e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5089,6 +5089,12 @@ S: Maintained F: drivers/gpu/drm/qxl/ F: include/uapi/drm/qxl_drm.h +DRM DRIVER FOR RAYDIUM RM67191 PANELS +M: Robert Chiras +S: Maintained +F: drivers/gpu/drm/panel/panel-raydium-rm67191.c +F: Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt + DRM DRIVER FOR RAGE 128 VIDEO CARDS S: Orphan / Obsolete F: drivers/gpu/drm/r128/ diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index d9d931a..8be1ac1 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -159,6 +159,15 @@ config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN Pi 7" Touchscreen. To compile this driver as a module, choose M here. +config DRM_PANEL_RAYDIUM_RM67191 + tristate "Raydium RM67191 FHD 1080x1920 DSI video mode panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for Raydium RM67191 FHD + (1080x1920) DSI panel. + config DRM_PANEL_RAYDIUM_RM68200 tristate "Raydium RM68200 720x1280 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index fb0cb3a..1fc0f68 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o +obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o obj-$(CONFIG_DRM_PANEL_ROCKTECH_JH057N00900) += panel-rocktech-jh057n00900.o obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c new file mode 100644 index 000..1735b0f --- /dev/null +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c @@ -0,0 +1,673 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raydium RM67191 MIPI-DSI panel driver + * + * Copyright 2019 NXP + */ + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include + +/* Panel specific color-format bits */ +#define COL_FMT_16BPP 0x55 +#define COL_FMT_18BPP 0x66 +#define COL_FMT_24BPP 0x77 + +/* Write Manufacture Command Set Control */ +#define WRMAUCCTR 0xFE + +/* Manufacturer Command Set pages (CMD2) */ +struct cmd_set_entry { + u8 cmd; + u8 param; +}; + +/* + * There is no description in the Reference Manual about these commands. + * We received them from vendor, so just use them as is. + */ +static const struct cmd_set_entry manufacturer_cmd_set[] = { + {0xFE, 0x0B}, + {0x28, 0x40}, + {0x29, 0x4F}, + {0xFE, 0x0E}, + {0x4B, 0x00}, + {0x4C, 0x0F}, + {0x4D, 0x20}, + {0x4E, 0x40}, + {0x4F, 0x60}, + {0x50, 0xA0}, + {0x51, 0xC0}, + {0x52, 0xE0}, + {0x53, 0xFF}, + {0xFE, 0x0D}, + {0x18, 0x08}, + {0x42, 0x00}, + {0x08, 0x41}, + {0x46, 0x02}, + {0x72, 0x09}, + {0xFE, 0x0A}, + {0x24, 0x17}, + {0x04, 0x07}, + {0x1A, 0x0C}, + {0x0F, 0x44}, + {0xFE, 0x04}, + {0x00, 0x0C}, + {0x05, 0x08}, + {0x06, 0x08}, + {0x08, 0x08}, + {0x09, 0x08}, + {0x0A, 0xE6}, + {0x0B, 0x8C}, + {0x1A, 0x12}, + {0x1E, 0xE0}, + {0x29, 0x93}, + {0x2A, 0x93}, + {0x2F, 0x02}, + {0x31, 0x02}, + {0x33, 0x05}, + {0x37, 0x2D}, + {0x38, 0x2D}, + {0x3A, 0x1E}, + {0x3B, 0x1E}, + {0x3D, 0x27}, + {0x3F, 0x80}, + {0x40, 0x40}, + {0x41, 0xE0}, + {0x4F, 0x2F}, + {0x50, 0x1E}, + {0xFE, 0x06}, + {0x00, 0xCC}, + {0x05, 0x05}, + {0x07, 0xA2}, + {0x08, 0xCC}, + {0x0D, 0x03}, + {0x0F, 0xA2}, + {0x32, 0xCC}, + {0x37, 0x05}, + {0x39, 0x83}, + {0x3A, 0xCC}, + {0x41, 0x04}, + {0x43, 0x83}, + {0x44, 0xCC}, + {0x49, 0x05},
[PATCH v4 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel
Add dt-bindings documentation for Raydium RM67191 DSI panel. Signed-off-by: Robert Chiras Reviewed-by: Sam Ravnborg --- .../bindings/display/panel/raydium,rm67191.txt | 41 ++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt new file mode 100644 index 000..1042469 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt @@ -0,0 +1,41 @@ +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol + +Required properties: +- compatible: "raydium,rm67191" +- reg: virtual channel for MIPI-DSI protocol + must be <0> +- dsi-lanes: number of DSI lanes to be used + must be <3> or <4> +- port:input port node with endpoint definition as + defined in Documentation/devicetree/bindings/graph.txt; + the input port should be connected to a MIPI-DSI device + driver + +Optional properties: +- reset-gpios: a GPIO spec for the RST_B GPIO pin +- v3p3-supply: phandle to 3.3V regulator that powers the VDD_3V3 pin +- v1p8-supply: phandle to 1.8V regulator that powers the VDD_1V8 pin +- width-mm:see panel-common.txt +- height-mm: see panel-common.txt +- video-mode: 0 - burst-mode + 1 - non-burst with sync event + 2 - non-burst with sync pulse + +Example: + + panel@0 { + compatible = "raydium,rm67191"; + reg = <0>; + pinctrl-0 = <&pinctrl_mipi_dsi_0_1_en>; + pinctrl-names = "default"; + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; + dsi-lanes = <4>; + width-mm = <68>; + height-mm = <121>; + + port { + panel_in: endpoint { + remote-endpoint = <&mipi_out>; + }; + }; + }; -- 2.7.4
Re: [PATCH v2] drm/mcde: Fix uninitialized variable
On Tue, Jun 25, 2019 at 12:09:54AM +0200, Linus Walleij wrote: > On Tue, Jun 18, 2019 at 2:31 PM Dan Carpenter > wrote: > > > Thanks! > > Recording this as Acked-by: when applying. > That's is an honour for me. I figured that your Signed-off-by dwarfed my Ack. :P regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v10
Am 24.06.19 um 16:41 schrieb Daniel Vetter: > On Mon, Jun 24, 2019 at 03:58:00PM +0200, Christian König wrote: >> Am 24.06.19 um 13:23 schrieb Koenig, Christian: >>> Am 21.06.19 um 18:27 schrieb Daniel Vetter: >>> So I pondered a few ideas while working out: 1) We drop this filtering. Importer needs to keep track of all its mappings and filter out invalidates that aren't for that specific importer (either because already invalidated, or not yet mapped, or whatever). Feels fragile. [SNIP] >>> [SNIP] >>> >>> I will take a moment and look into #1 as well, but I still don't see the >>> need to change anything. >> That turned out much cleaner than I thought it would be. Essentially it is >> only a single extra line of code in amdgpu. >> >> Going to send that out as a patch set in a minute. > Yeah I mean kinda expected that because: > - everything's protected with ww_mutex anyway > - importer needs to keep track of mappings anways > So really all it needs to do is not be stupid and add the mapping it just > created to its tracking while still holding the ww_mutex. Similar on > invalidate/unmap. > > With that all we need is a huge note in the docs that importers need to > keep track of their mappings and dtrt (with all the examples here spelled > out in the appropriate kerneldoc). And then I'm happy :-) Should I also rename the invalidate callback into move_notify? Would kind of make sense since we are not necessary directly invalidating mappings. Christian. > > Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 04/15] mm: untag user pointers passed to memory syscalls
This patch is a part of a series that extends 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, move_pages. The mmap and mremap syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma. Reviewed-by: Khalid Aziz Reviewed-by: Vincenzo Frascino Reviewed-by: Catalin Marinas Reviewed-by: Kees Cook Signed-off-by: Andrey Konovalov --- mm/madvise.c | 2 ++ mm/mempolicy.c | 3 +++ mm/migrate.c | 2 +- mm/mincore.c | 2 ++ mm/mlock.c | 4 mm/mprotect.c | 2 ++ mm/mremap.c| 7 +++ mm/msync.c | 2 ++ 8 files changed, 23 insertions(+), 1 deletion(-) diff --git a/mm/madvise.c b/mm/madvise.c index 628022e674a7..39b82f8a698f 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -810,6 +810,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior) size_t len; struct blk_plug plug; + start = untagged_addr(start); + if (!madvise_behavior_valid(behavior)) return error; diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 01600d80ae01..78e0a88b2680 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -1360,6 +1360,7 @@ static long kernel_mbind(unsigned long start, unsigned long len, int err; unsigned short mode_flags; + start = untagged_addr(start); mode_flags = mode & MPOL_MODE_FLAGS; mode &= ~MPOL_MODE_FLAGS; if (mode >= MPOL_MAX) @@ -1517,6 +1518,8 @@ static int kernel_get_mempolicy(int __user *policy, int uninitialized_var(pval); nodemask_t nodes; + addr = untagged_addr(addr); + if (nmask != NULL && maxnode < nr_node_ids) return -EINVAL; diff --git a/mm/migrate.c b/mm/migrate.c index f2ecc2855a12..d22c45cf36b2 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1616,7 +1616,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes, goto out_flush; if (get_user(node, nodes + i)) goto out_flush; - addr = (unsigned long)p; + addr = (unsigned long)untagged_addr(p); err = -ENODEV; if (node < 0 || node >= MAX_NUMNODES) diff --git a/mm/mincore.c b/mm/mincore.c index c3f058bd0faf..64c322ed845c 100644 --- a/mm/mincore.c +++ b/mm/mincore.c @@ -249,6 +249,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, len, unsigned long pages; unsigned char *tmp; + start = untagged_addr(start); + /* Check the start address: needs to be page-aligned.. */ if (start & ~PAGE_MASK) return -EINVAL; diff --git a/mm/mlock.c b/mm/mlock.c index a90099da4fb4..a72c1eeded77 100644 --- a/mm/mlock.c +++ b/mm/mlock.c @@ -674,6 +674,8 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla unsigned long lock_limit; int error = -ENOMEM; + start = untagged_addr(start); + if (!can_do_mlock()) return -EPERM; @@ -735,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len) { int ret; + start = untagged_addr(start); + len = PAGE_ALIGN(len + (offset_in_page(start))); start &= PAGE_MASK; diff --git a/mm/mprotect.c b/mm/mprotect.c index bf38dfbbb4b4..19f981b733bc 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -465,6 +465,8 @@ static int do_mprotect_pkey(unsigned long start, size_t len, const bool rier = (current->personality & READ_IMPLIES_EXEC) && (prot & PROT_READ); + start = untagged_addr(start); + prot &= ~(PROT_GROWSDOWN|PROT_GROWSUP); if (grows == (PROT_GROWSDOWN|PROT_GROWSUP)) /* can't be both */ return -EINVAL; diff --git a/mm/mremap.c b/mm/mremap.c index fc241d23cd97..64c9a3b8be0a 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -606,6 +606,13 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len, LIST_HEAD(uf_unmap_early); LIST_HEAD(uf_unmap); + /* +* Architectures may interpret the tag passed to mmap as a background +* colour for the corresponding vma. For mremap we don't allow tagged +* new_addr to preserve similar behaviour to mmap. +*/ + addr = untagged_addr(addr); + if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE)) return ret; diff --git a/mm/msync.c b/mm/msync.c index ef30a429623a..c3bd3e75f687 100644 --- a/mm/msync.c +++ b/mm/msync.c @@ -37,6 +37,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags) int unmapped_error = 0; int error = -EINVAL; + start = untagged_addr(start); +
[PATCH v4 hmm 06/12] mm/hmm: Do not use list*_rcu() for hmm->ranges
From: Jason Gunthorpe This list is always read and written while holding hmm->lock so there is no need for the confusing _rcu annotations. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: John Hubbard Acked-by: Souptick Joarder Reviewed-by: Ralph Campbell Reviewed-by: Ira Weiny Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- mm/hmm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 0423f4ca3a7e09..73c8af4827fe87 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -912,7 +912,7 @@ int hmm_range_register(struct hmm_range *range, range->hmm = hmm; kref_get(&hmm->kref); - list_add_rcu(&range->list, &hmm->ranges); + list_add(&range->list, &hmm->ranges); /* * If there are any concurrent notifiers we have to wait for them for @@ -942,7 +942,7 @@ void hmm_range_unregister(struct hmm_range *range) return; mutex_lock(&hmm->lock); - list_del_rcu(&range->list); + list_del(&range->list); mutex_unlock(&hmm->lock); /* Drop reference taken by hmm_range_register() */ -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 05/15] mm: untag user pointers in mm/gup.c
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. mm/gup.c provides a kernel interface that accepts user addresses and manipulates user pages directly (for example get_user_pages, that is used by the futex syscall). Since a user can provided tagged addresses, we need to handle this case. Add untagging to gup.c functions that use user addresses for vma lookups. Reviewed-by: Khalid Aziz Reviewed-by: Vincenzo Frascino Reviewed-by: Kees Cook Reviewed-by: Catalin Marinas Signed-off-by: Andrey Konovalov --- mm/gup.c | 4 1 file changed, 4 insertions(+) diff --git a/mm/gup.c b/mm/gup.c index ddde097cf9e4..c37df3d455a2 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -802,6 +802,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm, if (!nr_pages) return 0; + start = untagged_addr(start); + VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET)); /* @@ -964,6 +966,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm, struct vm_area_struct *vma; vm_fault_t ret, major = 0; + address = untagged_addr(address); + if (unlocked) fault_flags |= FAULT_FLAG_ALLOW_RETRY; -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 14/15] vfio/type1: untag user pointers in vaddr_get_pfn
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. vaddr_get_pfn() uses provided user pointers for vma lookups, which can only by done with untagged pointers. Untag user pointers in this function. Reviewed-by: Eric Auger Reviewed-by: Vincenzo Frascino Reviewed-by: Catalin Marinas Reviewed-by: Kees Cook Signed-off-by: Andrey Konovalov --- drivers/vfio/vfio_iommu_type1.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index add34adfadc7..7b8283e33d10 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -381,6 +381,8 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr, down_read(&mm->mmap_sem); + vaddr = untagged_addr(vaddr); + vma = find_vma_intersection(mm, vaddr, vaddr + 1); if (vma && vma->vm_flags & VM_PFNMAP) { -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
On Mon, Jun 24, 2019 at 04:32:52PM +0200, Andrey Konovalov wrote: > This patch is a part of a series that extends 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: Khalid Aziz > Reviewed-by: Vincenzo Frascino > Reviewed-by: Kees Cook > Reviewed-by: Catalin Marinas > Signed-off-by: Andrey Konovalov > --- > fs/namespace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 7660c2749c96..ec78f7223917 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -2994,7 +2994,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; I think this patch needs an ack from Al Viro (cc'ed). -- Catalin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Mon, Jun 24, 2019 at 04:33:00PM +0200, Andrey Konovalov wrote: > --- /dev/null > +++ b/tools/testing/selftests/arm64/tags_test.c > @@ -0,0 +1,29 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56) > +#define SET_TAG(ptr, tag)(((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \ > + SHIFT_TAG(tag)) > + > +int main(void) > +{ > + static int tbi_enabled = 0; > + struct utsname *ptr, *tagged_ptr; > + int err; > + > + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) > + tbi_enabled = 1; Nitpick: with the latest prctl() patch, you can skip the last three arguments as they are ignored. Either way: Reviewed-by: Catalin Marinas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 09/12] mm/hmm: Remove racy protection against double-unregistration
From: Jason Gunthorpe No other register/unregister kernel API attempts to provide this kind of protection as it is inherently racy, so just drop it. Callers should provide their own protection, and it appears nouveau already does. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v3 - Drop poison, looks like there are no new patches that will use this wrong (Christoph) --- mm/hmm.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 6f5dc6d568feb1..2ef14b2b5505f6 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -276,17 +276,11 @@ EXPORT_SYMBOL(hmm_mirror_register); */ void hmm_mirror_unregister(struct hmm_mirror *mirror) { - struct hmm *hmm = READ_ONCE(mirror->hmm); - - if (hmm == NULL) - return; + struct hmm *hmm = mirror->hmm; down_write(&hmm->mirrors_sem); list_del_init(&mirror->list); - /* To protect us against double unregister ... */ - mirror->hmm = NULL; up_write(&hmm->mirrors_sem); - hmm_put(hmm); } EXPORT_SYMBOL(hmm_mirror_unregister); -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 10/12] mm/hmm: Poison hmm_range during unregister
From: Jason Gunthorpe Trying to misuse a range outside its lifetime is a kernel bug. Use poison bytes to help detect this condition. Double unregister will reliably crash. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: John Hubbard Acked-by: Souptick Joarder Reviewed-by: Ralph Campbell Reviewed-by: Ira Weiny Tested-by: Philip Yang --- v2 - Keep range start/end valid after unregistration (Jerome) v3 - Revise some comments (John) - Remove start/end WARN_ON (Souptick) v4 - Fix tabs vs spaces in comment (Christoph) --- mm/hmm.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 2ef14b2b5505f6..c30aa9403dbe4d 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -925,19 +925,21 @@ void hmm_range_unregister(struct hmm_range *range) { struct hmm *hmm = range->hmm; - /* Sanity check this really should not happen. */ - if (hmm == NULL || range->end <= range->start) - return; - mutex_lock(&hmm->lock); list_del_init(&range->list); mutex_unlock(&hmm->lock); /* Drop reference taken by hmm_range_register() */ - range->valid = false; mmput(hmm->mm); hmm_put(hmm); - range->hmm = NULL; + + /* +* The range is now invalid and the ref on the hmm is dropped, so +* poison the pointer. Leave other fields in place, for the caller's +* use. +*/ + range->valid = false; + memset(&range->hmm, POISON_INUSE, sizeof(range->hmm)); } EXPORT_SYMBOL(hmm_range_unregister); -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] dt-bindings: backlight: fix vendor prefix for ArcticSand arcxcnn driver bindings
The vendor-prefixes.txt file properly refers to ArcticSand as arctic but the driver bindings improperly abbreviated the prefix to arc. This was a mistake in the original patch Signed-off-by: Brian Dodge --- .../bindings/leds/backlight/arcxcnn_bl.txt | 24 +- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt index 230abde..9cf4c44 100644 --- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt @@ -1,8 +1,12 @@ -Binding for ArcticSand arc2c0608 LED driver +Binding for ArcticSand arc family LED drivers Required properties: -- compatible: should be "arc,arc2c0608" -- reg: slave address +- compatible: one of + "arctic,arc1c0608" + "arctic,arc2c0608" + "arctic,arc3c0845" + +- reg: slave address Optional properties: - default-brightness: brightness value on boot, value from: 0-4095 @@ -11,19 +15,19 @@ Optional properties: - led-sources: List of enabled channels from 0 to 5. See Documentation/devicetree/bindings/leds/common.txt -- arc,led-config-0:setting for register ILED_CONFIG_0 -- arc,led-config-1:setting for register ILED_CONFIG_1 -- arc,dim-freq:PWM mode frequence setting (bits [3:0] used) -- arc,comp-config: setting for register CONFIG_COMP -- arc,filter-config: setting for register FILTER_CONFIG -- arc,trim-config: setting for register IMAXTUNE +- arctic,led-config-0: setting for register ILED_CONFIG_0 +- arctic,led-config-1: setting for register ILED_CONFIG_1 +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used) +- arctic,comp-config: setting for register CONFIG_COMP +- arctic,filter-config:setting for register FILTER_CONFIG +- arctic,trim-config: setting for register IMAXTUNE Note: Optional properties not specified will default to values in IC EPROM Example: arc2c0608@30 { - compatible = "arc,arc2c0608"; + compatible = "arctic,arc2c0608"; reg = <0x30>; default-brightness = <500>; label = "lcd-backlight"; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/9] drm: meson: vpp: use proper macros instead of magic constants
This patch add new macros which are used to set the following registers: - VPP_OSD_SCALE_COEF_IDX - VPP_DOLBY_CTRL - VPP_OFIFO_SIZE - VPP_HOLD_LINES - VPP_SC_MISC - VPP_VADJ_CTRL Signed-off-by: Julien Masson --- drivers/gpu/drm/meson/meson_registers.h | 8 drivers/gpu/drm/meson/meson_vpp.c | 27 - 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h index a9db49e5bdd6..172f0794e0cd 100644 --- a/drivers/gpu/drm/meson/meson_registers.h +++ b/drivers/gpu/drm/meson/meson_registers.h @@ -349,6 +349,7 @@ #define VPP_LINE_IN_LENGTH 0x1d01 #define VPP_PIC_IN_HEIGHT 0x1d02 #define VPP_SCALE_COEF_IDX 0x1d03 +#defineVPP_SCALE_HORIZONTAL_COEF BIT(8) #define VPP_SCALE_COEF 0x1d04 #define VPP_VSC_REGION12_STARTP 0x1d05 #define VPP_VSC_REGION34_STARTP 0x1d06 @@ -385,6 +386,8 @@ #define VPP_PREBLEND_H_SIZE 0x1d20 #define VPP_POSTBLEND_H_SIZE 0x1d21 #define VPP_HOLD_LINES 0x1d22 +#defineVPP_POSTBLEND_HOLD_LINES(lines) (lines & 0xf) +#defineVPP_PREBLEND_HOLD_LINES(lines) ((lines & 0xf) << 8) #define VPP_BLEND_ONECOLOR_CTRL 0x1d23 #define VPP_PREBLEND_CURRENT_XY 0x1d24 #define VPP_POSTBLEND_CURRENT_XY 0x1d25 @@ -403,6 +406,8 @@ #defineVPP_OSD2_PREBLEND BIT(17) #defineVPP_COLOR_MNG_ENABLEBIT(28) #define VPP_OFIFO_SIZE 0x1d27 +#defineVPP_OFIFO_SIZE_MASK GENMASK(13, 0) +#defineVPP_OFIFO_SIZE_DEFAULT (0xfff << 20 | 0x1000) #define VPP_FIFO_STATUS 0x1d28 #define VPP_SMOKE_CTRL 0x1d29 #define VPP_SMOKE1_VAL 0x1d2a @@ -418,6 +423,8 @@ #define VPP_HSC_PHASE_CTRL1 0x1d34 #define VPP_HSC_INI_PAT_CTRL 0x1d35 #define VPP_VADJ_CTRL 0x1d40 +#defineVPP_MINUS_BLACK_LVL_VADJ1_ENABLE BIT(1) + #define VPP_VADJ1_Y 0x1d41 #define VPP_VADJ1_MA_MB 0x1d42 #define VPP_VADJ1_MC_MD 0x1d43 @@ -477,6 +484,7 @@ #define VPP_PEAKING_VGAIN 0x1d92 #define VPP_PEAKING_NLP_1 0x1d93 #define VPP_DOLBY_CTRL 0x1d93 +#define VPP_PPS_DUMMY_DATA_MODE (1 << 17) #define VPP_PEAKING_NLP_2 0x1d94 #define VPP_PEAKING_NLP_3 0x1d95 #define VPP_PEAKING_NLP_4 0x1d96 diff --git a/drivers/gpu/drm/meson/meson_vpp.c b/drivers/gpu/drm/meson/meson_vpp.c index bfee30fa6e34..c2aaf81b0101 100644 --- a/drivers/gpu/drm/meson/meson_vpp.c +++ b/drivers/gpu/drm/meson/meson_vpp.c @@ -57,7 +57,7 @@ static void meson_vpp_write_scaling_filter_coefs(struct meson_drm *priv, { int i; - writel_relaxed(is_horizontal ? BIT(8) : 0, + writel_relaxed(is_horizontal ? VPP_SCALE_HORIZONTAL_COEF : 0, priv->io_base + _REG(VPP_OSD_SCALE_COEF_IDX)); for (i = 0; i < 33; i++) writel_relaxed(coefs[i], @@ -82,7 +82,7 @@ static void meson_vpp_write_vd_scaling_filter_coefs(struct meson_drm *priv, { int i; - writel_relaxed(is_horizontal ? BIT(8) : 0, + writel_relaxed(is_horizontal ? VPP_SCALE_HORIZONTAL_COEF : 0, priv->io_base + _REG(VPP_SCALE_COEF_IDX)); for (i = 0; i < 33; i++) writel_relaxed(coefs[i], @@ -97,20 +97,22 @@ void meson_vpp_init(struct meson_drm *priv) else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu")) { writel_bits_relaxed(0xff << 16, 0xff << 16, priv->io_base + _REG(VIU_MISC_CTRL1)); - writel_relaxed(0x2, priv->io_base + _REG(VPP_DOLBY_CTRL)); - writel_relaxed(0x1020080, + writel_relaxed(VPP_PPS_DUMMY_DATA_MODE, + priv->io_base + _REG(VPP_DOLBY_CTRL)); + writel_relaxed(0x108080, priv->io_base + _REG(VPP_DUMMY_DATA1)); } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) writel_relaxed(0xf, priv->io_base + _REG(DOLBY_PATH_CTRL)); /* Initialize vpu fifo control registers */ if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) - writel_relaxed(0xfff << 20 | 0x1000, + writel_relaxed(VPP_OFIFO_SIZE_DEFAULT, priv->io_base + _REG(VPP_OFIFO_SIZE)); else - writel_relaxed(readl_relaxed(priv->io_base + _REG(VPP_OFIFO_SIZE)) | - 0x77f, priv->io_base + _REG(VPP_OFIFO_SIZE)); - writel_relaxed(0x08080808, priv->io_base + _REG(VPP_HOLD_LINES)); + writel_bits_relaxed(VPP_OFIFO_SIZE_MASK, 0x77f, + priv->io_base + _REG(VPP_OFIFO_SIZE)); + writel_relaxed(VPP_POSTBLEND_HOLD_LINES(4) | VPP_PREBLEND_HOLD_LINES(4), + priv->io_base + _REG(VPP_HOLD_LINES)); if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) { /* Turn off preblend */ @@ -138,10 +140,15 @@ void meson_vpp_init(
[PATCH 0/2] fix vendor prefix for arcxcnn driver and bindings
These two patches supercede the prior similar three-patch set submitted on 6 Mov 2018. Apologies for the confusion. This patch is to update the arcxcnn backlight driver to use the proper "arctic" vendor-prefix and document that in the device- tree bindings. There is at least one existing device using the old "arc" vendor-prefix (Samsung Chromebook Plus), so support for that remains in the driver source. Unlike the previous patch set which hasn't been applied, there there is no actual functionality change here. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > In order to support additional features, rename hex_dump_to_buffer > > to > > hex_dump_to_buffer_ext, and replace the ascii bool parameter with > > flags. > [] > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > b/drivers/gpu/drm/i915/intel_engine_cs.c > [] > > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, > > const void *buf, size_t len) > > } > > > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, > > - rowsize, sizeof(u32), > > - line, sizeof(line), > > - false) >= > > sizeof(line)); > > + rowsize, sizeof(u32), > > line, > > + sizeof(line)) >= > > sizeof(line)); > > Huh? Why do this? > > > diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c > > b/drivers/isdn/hardware/mISDN/mISDNisar.c > [] > > @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, > > u8 len, u8 *msg) > > int l = 0; > > > > while (l < (int)len) { > > - hex_dump_to_buffer(msg + l, len - l, > > 32, 1, > > - isar->log, 256, 1); > > + hex_dump_to_buffer_ext(msg + l, len - > > l, 32, 1, > > + isar->log, 256, > > + HEXDUMP_ASCII); > > Again, why do any of these? > > The point of the wrapper is to avoid changing these. > > The change actions Jani's suggestion: https://lkml.org/lkml/2019/6/20/343 -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
From: Alastair D'Silva With the wider display format, it can become hard to identify how many bytes into the line you are looking at. The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to print vertical lines to separate every N groups of bytes. eg. buf:: 454d414e 43415053|4e495f45 00584544 NAMESPAC|E_INDEX. buf:0010: 0002| | Signed-off-by: Alastair D'Silva --- include/linux/printk.h | 3 +++ lib/hexdump.c | 59 -- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index f0761b3a2d5d..ae80d7af82ab 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -485,6 +485,9 @@ enum { #define HEXDUMP_ASCII BIT(0) #define HEXDUMP_SUPPRESS_REPEATED BIT(1) +#define HEXDUMP_2_GRP_LINESBIT(2) +#define HEXDUMP_4_GRP_LINESBIT(3) +#define HEXDUMP_8_GRP_LINESBIT(4) extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, diff --git a/lib/hexdump.c b/lib/hexdump.c index 1bf838c1a568..4dcf759fe048 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -77,6 +77,23 @@ char *bin2hex(char *dst, const void *src, size_t count) } EXPORT_SYMBOL(bin2hex); +static const char *group_separator(int group, u64 flags) +{ + if (group == 0) + return " "; + + if ((flags & HEXDUMP_8_GRP_LINES) && !((group) % 8)) + return "|"; + + if ((flags & HEXDUMP_4_GRP_LINES) && !((group) % 4)) + return "|"; + + if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2)) + return "|"; + + return " "; +} + /** * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory * @buf: data blob to dump @@ -87,6 +104,9 @@ EXPORT_SYMBOL(bin2hex); * @linebuflen: total size of @linebuf, including space for terminating NUL * @flags: A bitwise OR of the following flags: * HEXDUMP_ASCII: include ASCII after the hex output + * HEXDUMP_2_GRP_LINES:insert a '|' after every 2 groups + * HEXDUMP_4_GRP_LINES:insert a '|' after every 4 groups + * HEXDUMP_8_GRP_LINES:insert a '|' after every 8 groups * * hex_dump_to_buffer() works on one "line" of output at a time, converting * bytes of input to hexadecimal (and optionally printable ASCII) @@ -119,6 +139,7 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, int j, lx = 0; int ascii_column; int ret; + int line_chars = 0; if (!is_power_of_2(groupsize) || groupsize > 8) groupsize = 1; @@ -145,7 +166,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, for (j = 0; j < ngroups; j++) { ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%16.16llx", j ? " " : "", + "%s%16.16llx", + j ? group_separator(j, flags) : "", get_unaligned(ptr8 + j)); if (ret >= linebuflen - lx) goto overflow1; @@ -156,7 +178,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, for (j = 0; j < ngroups; j++) { ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%8.8x", j ? " " : "", + "%s%8.8x", + j ? group_separator(j, flags) : "", get_unaligned(ptr4 + j)); if (ret >= linebuflen - lx) goto overflow1; @@ -167,7 +190,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, for (j = 0; j < ngroups; j++) { ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%4.4x", j ? " " : "", + "%s%4.4x", + j ? group_separator(j, flags) : "", get_unaligned(ptr2 + j)); if (ret >= linebuflen - lx) goto overflow1; @@ -197,11 +221,26 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, goto overflow2; linebuf[lx++] = ' '; } + + if (flags & HEXDUMP_2_GRP_LINES) + line_chars = groupsize * 2; + if (flags & HEXDUMP_4_GRP_LINES) + line_chars = groupsize * 4; + if (flags & HEXDUMP_8_GRP_LINES) + line_chars = groupsize * 8; + for (j = 0; j < len; j++) {
[PATCH v4 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
From: Alastair D'Silva Some buffers may only be partially filled with useful data, while the rest is padded (typically with 0x00 or 0xff). This patch introduces a flag to allow the supression of lines of repeated bytes, which are replaced with '** Skipped %u bytes of value 0x%x **' An inline wrapper function is provided for backwards compatibility with existing code, which maintains the original behaviour. Signed-off-by: Alastair D'Silva --- include/linux/printk.h | 26 +--- lib/hexdump.c | 91 -- 2 files changed, 100 insertions(+), 17 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index cefd374c47b1..c0416d0eb9e2 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -7,6 +7,7 @@ #include #include #include +#include extern const char linux_banner[]; extern const char linux_proc_banner[]; @@ -481,13 +482,18 @@ enum { DUMP_PREFIX_ADDRESS, DUMP_PREFIX_OFFSET }; + extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, bool ascii); + +#define HEXDUMP_ASCII BIT(0) +#define HEXDUMP_SUPPRESS_REPEATED BIT(1) + #ifdef CONFIG_PRINTK -extern void print_hex_dump(const char *level, const char *prefix_str, +extern void print_hex_dump_ext(const char *level, const char *prefix_str, int prefix_type, int rowsize, int groupsize, - const void *buf, size_t len, bool ascii); + const void *buf, size_t len, u32 flags); #if defined(CONFIG_DYNAMIC_DEBUG) #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\ dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true) @@ -496,18 +502,28 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len); #endif /* defined(CONFIG_DYNAMIC_DEBUG) */ #else -static inline void print_hex_dump(const char *level, const char *prefix_str, +static inline void print_hex_dump_ext(const char *level, const char *prefix_str, int prefix_type, int rowsize, int groupsize, - const void *buf, size_t len, bool ascii) + const void *buf, size_t len, u32 flags) { } static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len) { } - #endif +static __always_inline void print_hex_dump(const char *level, + const char *prefix_str, + int prefix_type, int rowsize, + int groupsize, const void *buf, + size_t len, bool ascii) +{ + print_hex_dump_ext(level, prefix_str, prefix_type, rowsize, groupsize, + buf, len, ascii ? HEXDUMP_ASCII : 0); +} + + #if defined(CONFIG_DYNAMIC_DEBUG) #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ groupsize, buf, len, ascii)\ diff --git a/lib/hexdump.c b/lib/hexdump.c index 870c8cbff1e1..61dc625c32f5 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, EXPORT_SYMBOL(hex_dump_to_buffer); #ifdef CONFIG_PRINTK + +/** + * buf_is_all - Check if a buffer contains only a single byte value + * @buf: pointer to the buffer + * @len: the size of the buffer in bytes + * @val: outputs the value if if the bytes are identical + */ +static bool buf_is_all(const u8 *buf, size_t len, u8 *val_out) +{ + size_t i; + u8 val; + + if (len <= 1) + return false; + + val = buf[0]; + + for (i = 1; i < len; i++) { + if (buf[i] != val) + return false; + } + + *val_out = val; + return true; +} + +static void announce_skipped(const char *level, const char *prefix_str, + u8 val, size_t count) +{ + if (count == 0) + return; + + printk("%s%s ** Skipped %lu bytes of value 0x%x **\n", + level, prefix_str, count, val); +} + /** - * print_hex_dump - print a text hex dump to syslog for a binary blob of data + * print_hex_dump_ext - dump a binary blob of data to syslog in hexadecimal * @level: kernel log level (e.g. KERN_DEBUG) * @prefix_str: string to prefix each line with; * caller supplies trailing spaces for alignment if desired @@ -224,6 +260,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * @buf: data blob to dump * @len: number of bytes in the @buf * @ascii: include ASCII after the hex output + * @flags: A bitwise OR of the following flags: + * HEXDUMP
Re: [PATCH 2/3] backlight/arcxcnn fix vendor prefix
This sure did fall through the cracks. I confirmed with the vendor that there are no existing embedded DTS with the wrong name(s) in them before submitting this patch. The new owner of this chip family, pSemi, just wanted me to wrap things up and support all of there chips (3) in a single driver and that was the extent of the work for me. Since then the manager of the program there has also changed. I assume they'd still want these changes in for completeness. AFAIK, there were just some quibbles about the copyright date range. Can you please help me push these patches in? It'll take me some time to get back in to where I left things since its been so long. I know its a bit messy but the DTS and driver changes need to be together to make sense so I couldn't really do an incremental patch sequence. What is the next step? Brian On 6/24/19 6:24 AM, Daniel Thompson wrote: On Sat, Jun 22, 2019 at 12:13:25AM +0200, Pavel Machek wrote: Hi! [Sorry to those receiving this twice... had to dig this out from the archives and sent it to the lists from the wrong mailer] On 27/11/2018 00:44, Brian Dodge wrote: Thank you Pavel, that is a good point. The chip vendor has indicated that there is no reason to maintain the old/improper prefix and wishes to go forward (only) with the "arctic" prefix and any existing dts files are or will be updated. Looks like this patch series has fallen into the cracks a little. I think I assumed this info would end in the description of patch v2 1/3 (in order to answer Rob's feedback) and I sat and waited for a respin. On the other hand... I didn't actually say that explicitly anywhere! So... I'd recommend a respin perhaps with a small bit of text explaining how the vendor can state that any existing dts files will be updated. This is a peripheral device so these strings are probably embedded into OEM devicetrees rather than exclusively under the control of the vendor. So in next email you give good reason not to apply this :-). Afraid so... it was on page 2 of my google search so I did a quick search, sent the first mail and then went back to my web browser. It was at that moment that I decided a quick search wasn't enough and decided to got a little deeper! Daniel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 04/15] mm, arm64: untag user pointers passed to memory syscalls
On Wed, Jun 19, 2019 at 6:46 PM Khalid Aziz wrote: > > On 6/19/19 9:55 AM, Khalid Aziz wrote: > > On 6/12/19 5:43 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. > >> > >> This patch allows tagged pointers to be passed to the following memory > >> syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect, > >> mremap, msync, munlock, move_pages. > >> > >> The mmap and mremap syscalls do not currently accept tagged addresses. > >> Architectures may interpret the tag as a background colour for the > >> corresponding vma. > >> > >> Reviewed-by: Catalin Marinas > >> Reviewed-by: Kees Cook > >> Signed-off-by: Andrey Konovalov > >> --- > > > > Reviewed-by: Khalid Aziz > > > > > > I would also recommend updating commit log for all the patches in this > series that are changing files under mm/ as opposed to arch/arm64 to not > reference arm64 kernel ABI since the change applies to every > architecture. So something along the lines of "This patch is part of a > series that extends kernel ABI to allow..." Sure, will do in v18, thanks! > > -- > Khalid > > > >> mm/madvise.c | 2 ++ > >> mm/mempolicy.c | 3 +++ > >> mm/migrate.c | 2 +- > >> mm/mincore.c | 2 ++ > >> mm/mlock.c | 4 > >> mm/mprotect.c | 2 ++ > >> mm/mremap.c| 7 +++ > >> mm/msync.c | 2 ++ > >> 8 files changed, 23 insertions(+), 1 deletion(-) > >> > >> diff --git a/mm/madvise.c b/mm/madvise.c > >> index 628022e674a7..39b82f8a698f 100644 > >> --- a/mm/madvise.c > >> +++ b/mm/madvise.c > >> @@ -810,6 +810,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, > >> len_in, int, behavior) > >> size_t len; > >> struct blk_plug plug; > >> > >> +start = untagged_addr(start); > >> + > >> if (!madvise_behavior_valid(behavior)) > >> return error; > >> > >> diff --git a/mm/mempolicy.c b/mm/mempolicy.c > >> index 01600d80ae01..78e0a88b2680 100644 > >> --- a/mm/mempolicy.c > >> +++ b/mm/mempolicy.c > >> @@ -1360,6 +1360,7 @@ static long kernel_mbind(unsigned long start, > >> unsigned long len, > >> int err; > >> unsigned short mode_flags; > >> > >> +start = untagged_addr(start); > >> mode_flags = mode & MPOL_MODE_FLAGS; > >> mode &= ~MPOL_MODE_FLAGS; > >> if (mode >= MPOL_MAX) > >> @@ -1517,6 +1518,8 @@ static int kernel_get_mempolicy(int __user *policy, > >> int uninitialized_var(pval); > >> nodemask_t nodes; > >> > >> +addr = untagged_addr(addr); > >> + > >> if (nmask != NULL && maxnode < nr_node_ids) > >> return -EINVAL; > >> > >> diff --git a/mm/migrate.c b/mm/migrate.c > >> index f2ecc2855a12..d22c45cf36b2 100644 > >> --- a/mm/migrate.c > >> +++ b/mm/migrate.c > >> @@ -1616,7 +1616,7 @@ static int do_pages_move(struct mm_struct *mm, > >> nodemask_t task_nodes, > >> goto out_flush; > >> if (get_user(node, nodes + i)) > >> goto out_flush; > >> -addr = (unsigned long)p; > >> +addr = (unsigned long)untagged_addr(p); > >> > >> err = -ENODEV; > >> if (node < 0 || node >= MAX_NUMNODES) > >> diff --git a/mm/mincore.c b/mm/mincore.c > >> index c3f058bd0faf..64c322ed845c 100644 > >> --- a/mm/mincore.c > >> +++ b/mm/mincore.c > >> @@ -249,6 +249,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, > >> len, > >> unsigned long pages; > >> unsigned char *tmp; > >> > >> +start = untagged_addr(start); > >> + > >> /* Check the start address: needs to be page-aligned.. */ > >> if (start & ~PAGE_MASK) > >> return -EINVAL;fixup_user_fault > >> diff --git a/mm/mlock.c b/mm/mlock.c > >> index 080f3b36415b..e82609eaa428 100644 > >> --- a/mm/mlock.c > >> +++ b/mm/mlock.c > >> @@ -674,6 +674,8 @@ static __must_check int do_mlock(unsigned long start, > >> size_t len, vm_flags_t fla > >> unsigned long lock_limit; > >> int error = -ENOMEM; > >> > >> +start = untagged_addr(start); > >> + > >> if (!can_do_mlock()) > >> return -EPERM; > >> > >> @@ -735,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, > >> len) > >> { > >> int ret; > >> > >> +start = untagged_addr(start); > >> + > >> len = PAGE_ALIGN(len + (offset_in_page(start))); > >> start &= PAGE_MASK; > >> > >> diff --git a/mm/mprotect.c b/mm/mprotect.c > >> index bf38dfbbb4b4..19f981b733bc 100644 > >> --- a/mm/mprotect.c > >> +++ b/mm/mprotect.c > >> @@ -465,6 +465,8 @@ static int do_mprotect_pkey(unsigned long start, > >> size_t len, > >> const bool rier = (current->personality & READ_IMPLIES_EXEC) && > >> (prot & PROT_READ); > >> > >> +start = untagged_addr(start); > >> + > >> prot &= ~(PROT_GROWSDOWN|PROT_GROWSUP); > >> if (grows == (PROT
[PATCH v4 hmm 08/12] mm/hmm: Use lockdep instead of comments
From: Jason Gunthorpe So we can check locking at runtime. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Acked-by: Souptick Joarder Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v2 - Fix missing & in lockdeps (Jason) --- mm/hmm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 1eddda45cefae7..6f5dc6d568feb1 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -246,11 +246,11 @@ static const struct mmu_notifier_ops hmm_mmu_notifier_ops = { * * To start mirroring a process address space, the device driver must register * an HMM mirror struct. - * - * THE mm->mmap_sem MUST BE HELD IN WRITE MODE ! */ int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm) { + lockdep_assert_held_exclusive(&mm->mmap_sem); + /* Sanity check */ if (!mm || !mirror || !mirror->ops) return -EINVAL; -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 08/15] userfaultfd: untag user pointers
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. userfaultfd code use provided user pointers for vma lookups, which can only by done with untagged pointers. Untag user pointers in validate_range(). Reviewed-by: Vincenzo Frascino Reviewed-by: Catalin Marinas Reviewed-by: Kees Cook Signed-off-by: Andrey Konovalov --- fs/userfaultfd.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index ae0b8b5f69e6..c2be36a168ca 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx, } static __always_inline int validate_range(struct mm_struct *mm, - __u64 start, __u64 len) + __u64 *start, __u64 len) { __u64 task_size = mm->task_size; - if (start & ~PAGE_MASK) + *start = untagged_addr(*start); + + if (*start & ~PAGE_MASK) return -EINVAL; if (len & ~PAGE_MASK) return -EINVAL; if (!len) return -EINVAL; - if (start < mmap_min_addr) + if (*start < mmap_min_addr) return -EINVAL; - if (start >= task_size) + if (*start >= task_size) return -EINVAL; - if (len > task_size - start) + if (len > task_size - *start) return -EINVAL; return 0; } @@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, goto out; } - ret = validate_range(mm, uffdio_register.range.start, + ret = validate_range(mm, &uffdio_register.range.start, uffdio_register.range.len); if (ret) goto out; @@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister))) goto out; - ret = validate_range(mm, uffdio_unregister.start, + ret = validate_range(mm, &uffdio_unregister.start, uffdio_unregister.len); if (ret) goto out; @@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx, if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake))) goto out; - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len); + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len); if (ret) goto out; @@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, sizeof(uffdio_copy)-sizeof(__s64))) goto out; - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len); + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len); if (ret) goto out; /* @@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx, sizeof(uffdio_zeropage)-sizeof(__s64))) goto out; - ret = validate_range(ctx->mm, uffdio_zeropage.range.start, + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start, uffdio_zeropage.range.len); if (ret) goto out; -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva In order to support additional features, rename hex_dump_to_buffer to hex_dump_to_buffer_ext, and replace the ascii bool parameter with flags. A wrapper is provided for callers that do not need anything but a basic dump. Signed-off-by: Alastair D'Silva --- drivers/gpu/drm/i915/intel_engine_cs.c| 5 +- drivers/isdn/hardware/mISDN/mISDNisar.c | 10 ++-- drivers/mailbox/mailbox-test.c| 8 ++-- drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- .../net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- drivers/net/wireless/ath/ath10k/debug.c | 7 +-- .../net/wireless/intel/iwlegacy/3945-mac.c| 4 +- drivers/platform/chrome/wilco_ec/debugfs.c| 10 ++-- drivers/scsi/scsi_logging.c | 8 ++-- drivers/staging/fbtft/fbtft-core.c| 2 +- fs/seq_file.c | 6 ++- include/linux/printk.h| 46 +-- lib/hexdump.c | 24 +- lib/test_hexdump.c| 10 ++-- 14 files changed, 94 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index eea9bec04f1b..64189a0e5ec9 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len) } WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, - rowsize, sizeof(u32), - line, sizeof(line), - false) >= sizeof(line)); + rowsize, sizeof(u32), line, + sizeof(line)) >= sizeof(line)); drm_printf(m, "[%04zx] %s\n", pos, line); prev = buf + pos; diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c index fd5c52f37802..84455b521246 100644 --- a/drivers/isdn/hardware/mISDN/mISDNisar.c +++ b/drivers/isdn/hardware/mISDN/mISDNisar.c @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 *msg) int l = 0; while (l < (int)len) { - hex_dump_to_buffer(msg + l, len - l, 32, 1, - isar->log, 256, 1); + hex_dump_to_buffer_ext(msg + l, len - l, 32, 1, + isar->log, 256, + HEXDUMP_ASCII); pr_debug("%s: %s %02x: %s\n", isar->name, __func__, l, isar->log); l += 32; @@ -99,8 +100,9 @@ rcv_mbox(struct isar_hw *isar, u8 *msg) int l = 0; while (l < (int)isar->clsb) { - hex_dump_to_buffer(msg + l, isar->clsb - l, 32, - 1, isar->log, 256, 1); + hex_dump_to_buffer_ext(msg + l, isar->clsb - l, + 32, 1, isar->log, 256, + HEXDUMP_ASCII); pr_debug("%s: %s %02x: %s\n", isar->name, __func__, l, isar->log); l += 32; diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c index 4555d678fadd..ce334f88a3ee 100644 --- a/drivers/mailbox/mailbox-test.c +++ b/drivers/mailbox/mailbox-test.c @@ -206,10 +206,10 @@ static ssize_t mbox_test_message_read(struct file *filp, char __user *userbuf, ptr = tdev->rx_buffer; while (l < MBOX_HEXDUMP_MAX_LEN) { - hex_dump_to_buffer(ptr, - MBOX_BYTES_PER_LINE, - MBOX_BYTES_PER_LINE, 1, touser + l, - MBOX_HEXDUMP_LINE_LEN, true); + hex_dump_to_buffer_ext(ptr, + MBOX_BYTES_PER_LINE, + MBOX_BYTES_PER_LINE, 1, touser + l, + MBOX_HEXDUMP_LINE_LEN, HEXDUMP_ASCII); ptr += MBOX_BYTES_PER_LINE; l += MBOX_HEXDUMP_LINE_LEN; diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c index 3dd0cecddba8..f0118fe35c41 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c @@ -2992,7 +2992,7 @@ void xgbe_print_pkt(struct net_device *netdev, struct sk_buff *skb, bool tx_rx) unsigned int len = min(
[PATCH v18 01/15] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. copy_from_user (and a few other similar functions) are used to copy data from user memory into the kernel memory or vice versa. Since a user can provided a tagged pointer to one of the syscalls that use copy_from_user, we need to correctly handle such pointers. Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr, before performing access validity checks. Note, that this patch only temporarily untags the pointers to perform the checks, but then passes them as is into the kernel internals. Reviewed-by: Vincenzo Frascino Reviewed-by: Kees Cook Reviewed-by: Catalin Marinas Signed-off-by: Andrey Konovalov --- arch/arm64/include/asm/uaccess.h | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 5a1c32260c1f..a138e3b4f717 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -62,6 +62,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si { unsigned long ret, limit = current_thread_info()->addr_limit; + addr = untagged_addr(addr); + __chk_user_ptr(addr); asm volatile( // A + B <= C + 1 for all A,B,C, in four easy steps: @@ -215,7 +217,8 @@ static inline void uaccess_enable_not_uao(void) /* * Sanitise a uaccess pointer such that it becomes NULL if above the - * current addr_limit. + * current addr_limit. In case the pointer is tagged (has the top byte set), + * untag the pointer before checking. */ #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr) static inline void __user *__uaccess_mask_ptr(const void __user *ptr) @@ -223,10 +226,11 @@ static inline void __user *__uaccess_mask_ptr(const void __user *ptr) void __user *safe_ptr; asm volatile( - " bicsxzr, %1, %2\n" + " bicsxzr, %3, %2\n" " csel%0, %1, xzr, eq\n" : "=&r" (safe_ptr) - : "r" (ptr), "r" (current_thread_info()->addr_limit) + : "r" (ptr), "r" (current_thread_info()->addr_limit), + "r" (untagged_addr(ptr)) : "cc"); csdb(); -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged) userspace pointer. The untagged address should be used so that MMU notifiers for the untagged address get correctly matched up with the right BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses provided user pointers for vma lookups, which can only by done with untagged pointers. This patch untags user pointers in radeon_gem_userptr_ioctl(). Suggested-by: Felix Kuehling Acked-by: Felix Kuehling Signed-off-by: Andrey Konovalov --- drivers/gpu/drm/radeon/radeon_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 44617dec8183..90eb78fb5eb2 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + args->addr = untagged_addr(args->addr); + if (offset_in_page(args->addr | args->size)) return -EINVAL; -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 7/7] lib/hexdump.c: Optionally retain byte ordering
From: Alastair D'Silva The behaviour of hexdump groups is to print the data out as if it was a native-endian number. This patch tweaks the documentation to make this clear, and also adds the HEXDUMP_RETAIN_BYTE_ORDER flag to allow groups of multiple bytes to be printed without affecting the ordering of the printed bytes. Signed-off-by: Alastair D'Silva --- include/linux/printk.h | 1 + lib/hexdump.c | 30 lib/test_hexdump.c | 62 -- 3 files changed, 68 insertions(+), 25 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index 1d082291facf..ed1a79aa9695 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -491,6 +491,7 @@ enum { #define HEXDUMP_2_GRP_SPACES BIT(5) #define HEXDUMP_4_GRP_SPACES BIT(6) #define HEXDUMP_8_GRP_SPACES BIT(7) +#define HEXDUMP_RETAIN_BYTE_ORDER BIT(8) extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, diff --git a/lib/hexdump.c b/lib/hexdump.c index e09e3cf8e595..29024eccf5da 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -127,7 +127,8 @@ static void separator_parameters(u64 flags, int groupsize, int *sep_chars, * @buf: data blob to dump * @len: number of bytes in the @buf * @rowsize: number of bytes to print per line; must be a multiple of groupsize - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @groupsize: number of bytes to convert to a native endian number and print: + *1, 2, 4, 8; default = 1 * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL * @flags: A bitwise OR of the following flags: @@ -138,6 +139,9 @@ static void separator_parameters(u64 flags, int groupsize, int *sep_chars, * HEXDUMP_2_GRP_SPACES: insert a ' ' after every 2 groups * HEXDUMP_4_GRP_SPACES: insert a ' ' after every 4 groups * HEXDUMP_8_GRP_SPACES: insert a ' ' after every 8 groups + * HEXDUMP_RETAIN_BYTE_ORDER: Retain the byte ordering of groups + * instead of treating each group as a + * native-endian number * * hex_dump_to_buffer() works on one "line" of output at a time, converting * bytes of input to hexadecimal (and optionally printable ASCII) @@ -172,6 +176,7 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, int ret; int sep_chars = 0; char sep = 0; + bool big_endian = (flags & HEXDUMP_RETAIN_BYTE_ORDER) ? 1 : 0; if (!is_power_of_2(groupsize) || groupsize > 8) groupsize = 1; @@ -203,10 +208,13 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, const u64 *ptr8 = buf; for (j = 0; j < ngroups; j++) { + u64 val = big_endian ? + be64_to_cpu(get_unaligned(ptr8 + j)) : + get_unaligned(ptr8 + j); ret = snprintf(linebuf + lx, linebuflen - lx, "%s%16.16llx", j ? group_separator(j, flags) : "", - get_unaligned(ptr8 + j)); + val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -215,10 +223,14 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, const u32 *ptr4 = buf; for (j = 0; j < ngroups; j++) { + u32 val = big_endian ? + be32_to_cpu(get_unaligned(ptr4 + j)) : + get_unaligned(ptr4 + j); + ret = snprintf(linebuf + lx, linebuflen - lx, "%s%8.8x", j ? group_separator(j, flags) : "", - get_unaligned(ptr4 + j)); + val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -227,10 +239,14 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, const u16 *ptr2 = buf; for (j = 0; j < ngroups; j++) { + u16 val = big_endian ? + be16_to_cpu(get_unaligned(ptr2 + j)) : + get_unaligned(ptr2 + j); + ret = snprintf(linebuf + lx, linebuflen - lx, "%s%4.4x", j ? g
[PATCH v18 12/15] media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. videobuf_dma_contig_user_get() uses provided user pointers for vma lookups, which can only by done with untagged pointers. Untag the pointers in this function. Reviewed-by: Khalid Aziz Reviewed-by: Kees Cook Acked-by: Mauro Carvalho Chehab Signed-off-by: Andrey Konovalov --- drivers/media/v4l2-core/videobuf-dma-contig.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c b/drivers/media/v4l2-core/videobuf-dma-contig.c index 0491122b03c4..ec554eff29b9 100644 --- a/drivers/media/v4l2-core/videobuf-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf-dma-contig.c @@ -157,6 +157,7 @@ static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory *mem) static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem, struct videobuf_buffer *vb) { + unsigned long untagged_baddr = untagged_addr(vb->baddr); struct mm_struct *mm = current->mm; struct vm_area_struct *vma; unsigned long prev_pfn, this_pfn; @@ -164,22 +165,22 @@ static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem, unsigned int offset; int ret; - offset = vb->baddr & ~PAGE_MASK; + offset = untagged_baddr & ~PAGE_MASK; mem->size = PAGE_ALIGN(vb->size + offset); ret = -EINVAL; down_read(&mm->mmap_sem); - vma = find_vma(mm, vb->baddr); + vma = find_vma(mm, untagged_baddr); if (!vma) goto out_up; - if ((vb->baddr + mem->size) > vma->vm_end) + if ((untagged_baddr + mem->size) > vma->vm_end) goto out_up; pages_done = 0; prev_pfn = 0; /* kill warning */ - user_address = vb->baddr; + user_address = untagged_baddr; while (pages_done < (mem->size >> PAGE_SHIFT)) { ret = follow_pfn(vma, user_address, &this_pfn); -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote: > This patch is a part of a series that extends kernel ABI to allow to pass > tagged user pointers (with the top byte set to something else other than > 0x00) as syscall arguments. > > mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can > only by done with untagged pointers. > > Untag user pointers in this function. > > Signed-off-by: Andrey Konovalov > --- > drivers/infiniband/hw/mlx4/mr.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) Acked-by: Catalin Marinas This patch also needs an ack from the infiniband maintainers (Jason). -- Catalin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 11/12] mm/hmm: Remove confusing comment and logic from hmm_release
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(&hmm->mirrors_sem); hmm_mirror_unregister(mirror) down_write(&hmm->mirrors_sem); up_write(&hmm->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 Tested-by: Philip Yang --- mm/hmm.c | 28 +--- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index c30aa9403dbe4d..b224ea635a7716 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -130,26 +130,16 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm) */ WARN_ON(!list_empty_careful(&hmm->ranges)); - down_write(&hmm->mirrors_sem); - mirror = list_first_entry_or_null(&hmm->mirrors, struct hmm_mirror, - list); - while (mirror) { - list_del_init(&mirror->list); - if (mirror->ops->release) { - /* -* Drop mirrors_sem so the release callback can wait -* on any pending work that might itself trigger a -* mmu_notifier callback and thus would deadlock with -* us. -*/ - up_write(&hmm->mirrors_sem); + down_read(&hmm->mirrors_sem); + list_for_each_entry(mirror, &hmm->mirrors, list) { + /* +* Note: The driver is not allowed to trigger +* hmm_mirror_unregister() from this thread. +*/ + if (mirror->ops->release) mirror->ops->release(mirror); - down_write(&hmm->mirrors_sem); - } - mirror = list_first_entry_or_null(&hmm->mirrors, - struct hmm_mirror, list); } - up_write(&hmm->mirrors_sem); + up_read(&hmm->mirrors_sem); hmm_put(hmm); } @@ -279,7 +269,7 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror) struct hmm *hmm = mirror->hmm; down_write(&hmm->mirrors_sem); - list_del_init(&mirror->list); + list_del(&mirror->list); up_write(&hmm->mirrors_sem); hmm_put(hmm); } -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: Update Maintainers and Reviewers of DRM Bridge Drivers
Dne ponedeljek, 24. junij 2019 ob 11:08:51 CEST je Neil Armstrong napisal(a): > Add myself as co-maintainer of DRM Bridge Drivers then add Jonas Karlman > and Jernej Škrabec as Reviewers of DRM Bridge Drivers. > > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Andrzej Hajda > Cc: Jernej Škrabec > Cc: Daniel Vetter > Signed-off-by: Neil Armstrong Reviewed-by: Jernej Skrabec Thanks! Best regards, Jernej > --- > MAINTAINERS | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 2abf6d28db64..dd8dacc61e79 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5253,7 +5253,10 @@ T: git git://anongit.freedesktop.org/drm/drm- misc > > DRM DRIVERS FOR BRIDGE CHIPS > M: Andrzej Hajda > +M: Neil Armstrong > R: Laurent Pinchart > +R: Jonas Karlman > +R: Jernej Skrabec > S: Maintained > T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/bridge/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 13/15] tee/shm: untag user pointers in tee_shm_register
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided user pointers for vma lookups (via __check_mem_type()), which can only by done with untagged pointers. Untag user pointers in this function. Reviewed-by: Kees Cook Acked-by: Jens Wiklander Signed-off-by: Andrey Konovalov --- drivers/tee/tee_shm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 2da026fd12c9..09ddcd06c715 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -254,6 +254,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, shm->teedev = teedev; shm->ctx = ctx; shm->id = -1; + addr = untagged_addr(addr); start = rounddown(addr, PAGE_SIZE); shm->offset = addr - start; shm->size = length; -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
This patch is a part of a series that extends 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: Khalid Aziz Reviewed-by: Vincenzo Frascino Reviewed-by: Kees Cook Reviewed-by: Catalin Marinas Signed-off-by: Andrey Konovalov --- fs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/namespace.c b/fs/namespace.c index 7660c2749c96..ec78f7223917 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2994,7 +2994,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.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 03/15] lib: untag user pointers in strn*_user
This patch is a part of a series that extends 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: Vincenzo Frascino Reviewed-by: Khalid Aziz Acked-by: Kees Cook 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(-) 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; -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 00/15] arm64: untag user pointers passed to the kernel
=== Overview arm64 has a feature called Top Byte Ignore, which allows to embed pointer tags into the top byte of each pointer. Userspace programs (such as HWASan, a memory debugging tool [1]) might use this feature and pass tagged user pointers to the kernel through syscalls or other interfaces. Right now the kernel is already able to handle user faults with tagged pointers, due to these patches: 1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a tagged pointer") 2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged pointers") 3. 276e9327 ("arm64: entry: improve data abort handling of tagged pointers") This patchset extends tagged pointer support to syscall arguments. As per the proposed ABI change [3], tagged pointers are only allowed to be passed to syscalls when they point to memory ranges obtained by anonymous mmap() or sbrk() (see the patchset [3] for more details). For non-memory syscalls this is done by untaging user pointers when the kernel performs pointer checking to find out whether the pointer comes from userspace (most notably in access_ok). The untagging is done only when the pointer is being checked, the tag is preserved as the pointer makes its way through the kernel and stays tagged when the kernel dereferences the pointer when perfoming user memory accesses. The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma. Other memory syscalls (mprotect, etc.) don't do user memory accesses but rather deal with memory ranges, and untagged pointers are better suited to describe memory ranges internally. Thus for memory syscalls we untag pointers completely when they enter the kernel. === Other approaches One of the alternative approaches to untagging that was considered is to completely strip the pointer tag as the pointer enters the kernel with some kind of a syscall wrapper, but that won't work with the countless number of different ioctl calls. With this approach we would need a custom wrapper for each ioctl variation, which doesn't seem practical. An alternative approach to untagging pointers in memory syscalls prologues is to inspead allow tagged pointers to be passed to find_vma() (and other vma related functions) and untag them there. Unfortunately, a lot of find_vma() callers then compare or subtract the returned vma start and end fields against the pointer that was being searched. Thus this approach would still require changing all find_vma() callers. === Testing The following testing approaches has been taken to find potential issues with user pointer untagging: 1. Static testing (with sparse [2] and separately with a custom static analyzer based on Clang) to track casts of __user pointers to integer types to find places where untagging needs to be done. 2. Static testing with grep to find parts of the kernel that call find_vma() (and other similar functions) or directly compare against vm_start/vm_end fields of vma. 3. Static testing with grep to find parts of the kernel that compare user pointers with TASK_SIZE or other similar consts and macros. 4. Dynamic testing: adding BUG_ON(has_tag(addr)) to find_vma() and running a modified syzkaller version that passes tagged pointers to the kernel. Based on the results of the testing the requried patches have been added to the patchset. === Notes This patchset is meant to be merged together with "arm64 relaxed ABI" [3]. This patchset is a prerequisite for ARM's memory tagging hardware feature support [4]. This patchset has been merged into the Pixel 2 & 3 kernel trees and is now being used to enable testing of Pixel phones with HWASan. Thanks! [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html [2] https://github.com/lucvoo/sparse-dev/commit/5f960cb10f56ec2017c128ef9d16060e0145f292 [3] https://lkml.org/lkml/2019/6/12/745 [4] https://community.arm.com/processors/b/blog/posts/arm-a-profile-architecture-2018-developments-armv85a === History Changes in v18: - Reverted the selftest back to not using the LD_PRELOAD approach. - Added prctl(PR_SET_TAGGED_ADDR_CTRL) call to the selftest. - Reworded the patch descriptions to make them less oriented on arm64 only. - Catalin's patch: "I added a Kconfig option and dropped the prctl args zero check. There is some minor clean-up as well". Changes in v17: - The "uaccess: add noop untagged_addr definition" patch is dropped, as it was merged into upstream named as "uaccess: add noop untagged_addr definition". - Merged "mm, arm64: untag user pointers in do_pages_move" into "mm, arm64: untag user pointers passed to memory syscalls". - Added "arm64: Introduce prctl() options to control the tagged user addresses ABI" patch from Catalin. - Add tags_lib.so to tools/testing/selftests/arm64/.gitignore. - Added a comment clarifying untagged in
[PATCH 9/9] drm: meson: venc: set the correct macrovision max amplitude value
According to the register description of ENCI_MACV_MAX_AMP, the macrovision max amplitude value should be: - hdmi 480i => 0xb - hdmi 576i => 0x7 The max value is 0x7ff (10 bits). Signed-off-by: Julien Masson --- drivers/gpu/drm/meson/meson_venc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c index 2835133ab676..acad16ff7371 100644 --- a/drivers/gpu/drm/meson/meson_venc.c +++ b/drivers/gpu/drm/meson/meson_venc.c @@ -192,7 +192,7 @@ union meson_hdmi_venc_mode meson_hdmi_enci_mode_480i = { .hso_end = 129, .vso_even = 3, .vso_odd = 260, - .macv_max_amp = 0x810b, + .macv_max_amp = 0xb, .video_prog_mode = 0xf0, .video_mode = 0x8, .sch_adjust = 0x20, @@ -212,7 +212,7 @@ union meson_hdmi_venc_mode meson_hdmi_enci_mode_576i = { .hso_end = 129, .vso_even = 3, .vso_odd = 260, - .macv_max_amp = 8107, + .macv_max_amp = 0x7, .video_prog_mode = 0xff, .video_mode = 0x13, .sch_adjust = 0x28, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 12/12] mm/hmm: Fix error flows in hmm_invalidate_range_start
From: Jason Gunthorpe If the trylock on the hmm->mirrors_sem fails the function will return without decrementing the notifiers that were previously incremented. Since the caller will not call invalidate_range_end() on EAGAIN this will result in notifiers becoming permanently incremented and deadlock. If the sync_cpu_device_pagetables() required blocking the function will not return EAGAIN even though the device continues to touch the pages. This is a violation of the mmu notifier contract. Switch, and rename, the ranges_lock to a spin lock so we can reliably obtain it without blocking during error unwind. The error unwind is necessary since the notifiers count must be held incremented across the call to sync_cpu_device_pagetables() as we cannot allow the range to become marked valid by a parallel invalidate_start/end() pair while doing sync_cpu_device_pagetables(). Signed-off-by: Jason Gunthorpe Reviewed-by: Ralph Campbell Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- include/linux/hmm.h | 2 +- mm/hmm.c| 72 +++-- 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/include/linux/hmm.h b/include/linux/hmm.h index bf013e96525771..0fa8ea34ccef6d 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -86,7 +86,7 @@ struct hmm { struct mm_struct*mm; struct kref kref; - struct mutexlock; + spinlock_t ranges_lock; struct list_headranges; struct list_headmirrors; struct mmu_notifier mmu_notifier; diff --git a/mm/hmm.c b/mm/hmm.c index b224ea635a7716..89549eac03d506 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -64,7 +64,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm) init_rwsem(&hmm->mirrors_sem); hmm->mmu_notifier.ops = NULL; INIT_LIST_HEAD(&hmm->ranges); - mutex_init(&hmm->lock); + spin_lock_init(&hmm->ranges_lock); kref_init(&hmm->kref); hmm->notifiers = 0; hmm->mm = mm; @@ -144,6 +144,23 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm) hmm_put(hmm); } +static void notifiers_decrement(struct hmm *hmm) +{ + lockdep_assert_held(&hmm->ranges_lock); + + hmm->notifiers--; + if (!hmm->notifiers) { + struct hmm_range *range; + + list_for_each_entry(range, &hmm->ranges, list) { + if (range->valid) + continue; + range->valid = true; + } + wake_up_all(&hmm->wq); + } +} + static int hmm_invalidate_range_start(struct mmu_notifier *mn, const struct mmu_notifier_range *nrange) { @@ -151,6 +168,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn, struct hmm_mirror *mirror; struct hmm_update update; struct hmm_range *range; + unsigned long flags; int ret = 0; if (!kref_get_unless_zero(&hmm->kref)) @@ -161,12 +179,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn, update.event = HMM_UPDATE_INVALIDATE; update.blockable = mmu_notifier_range_blockable(nrange); - if (mmu_notifier_range_blockable(nrange)) - mutex_lock(&hmm->lock); - else if (!mutex_trylock(&hmm->lock)) { - ret = -EAGAIN; - goto out; - } + spin_lock_irqsave(&hmm->ranges_lock, flags); hmm->notifiers++; list_for_each_entry(range, &hmm->ranges, list) { if (update.end < range->start || update.start >= range->end) @@ -174,7 +187,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn, range->valid = false; } - mutex_unlock(&hmm->lock); + spin_unlock_irqrestore(&hmm->ranges_lock, flags); if (mmu_notifier_range_blockable(nrange)) down_read(&hmm->mirrors_sem); @@ -182,16 +195,26 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn, ret = -EAGAIN; goto out; } + list_for_each_entry(mirror, &hmm->mirrors, list) { - int ret; + int rc; - ret = mirror->ops->sync_cpu_device_pagetables(mirror, &update); - if (!update.blockable && ret == -EAGAIN) + rc = mirror->ops->sync_cpu_device_pagetables(mirror, &update); + if (rc) { + if (WARN_ON(update.blockable || rc != -EAGAIN)) + continue; + ret = -EAGAIN; break; + } } up_read(&hmm->mirrors_sem); out: + if (ret) { + spin_lock_irqsave(&hmm->ranges_lock, flags); + notifiers_decrement(hmm); + spin_unlock_irqrestore(&hmm->ranges_lock, flags); + } hmm
[PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
From: Catalin Marinas 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(). A Kconfig option allows the overall disabling of the relaxed ABI. 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 Signed-off-by: Andrey Konovalov --- arch/arm64/Kconfig | 9 arch/arm64/include/asm/processor.h | 8 arch/arm64/include/asm/thread_info.h | 1 + arch/arm64/include/asm/uaccess.h | 4 +- arch/arm64/kernel/process.c | 72 include/uapi/linux/prctl.h | 5 ++ kernel/sys.c | 12 + 7 files changed, 110 insertions(+), 1 deletion(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 697ea0510729..55fbaf20af2d 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1107,6 +1107,15 @@ config ARM64_SW_TTBR0_PAN zeroed area and reserved ASID. The user access routines restore the valid TTBR0_EL1 temporarily. +config ARM64_TAGGED_ADDR_ABI + bool "Enable the tagged user addresses syscall ABI" + default y + help + When this option is enabled, user applications can opt in to a + relaxed ABI via prctl() allowing tagged addresses to be passed + to system calls as pointer arguments. For details, see + Documentation/arm64/tagged-address-abi.txt. + menuconfig COMPAT bool "Kernel support for 32-bit EL0" depends on ARM64_4K_PAGES || EXPERT diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index fd5b1a4efc70..ee86070a28d4 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -296,6 +296,14 @@ extern void __init minsigstksz_setup(void); /* PR_PAC_RESET_KEYS prctl */ #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg) +#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI +/* PR_{SET,GET}_TAGGED_ADDR_CTRL 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() +#endif + /* * For CONFIG_GCC_PLUGIN_STACKLEAK * diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index 2372e97db29c..4f81c4f15404 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -88,6 +88,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 /* Allow tagged user addresses */ #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 a138e3b4f717..097d6bfac0b7 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -62,7 +62,9 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si { unsigned long ret, limit = current_thread_info()->addr_limit; - addr = untagged_addr(addr); + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && + test_thread_flag(TIF_TAGGED_ADDR)) + addr = untagged_addr(addr); __chk_user_ptr(addr); asm volatile( diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 9856395ccdb7..60e70158a4a1 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -307,11 +308,18 @@ static void tls_thread_flush(void) } } +static void flush_tagged_addr_state(void) +{ + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI)) + clear_thread_flag(TIF_TAGGED_ADDR); +} + void flush_thread(void) { fpsimd_flush_thread(); tls_thread_flush(); flush_ptrace_hw_breakpoint(current); + flush_tagged_addr_state(); } void release_thread(struct task_struct *dead_task) @@ -541,3 +549,67 @@ void arch_setup_new_exec(void) ptrauth_thread_init_user(current); } + +#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI +/* + * Control the relaxed ABI allowing tagged user addresses into the kernel. + *
[PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
This patch is a part of a series that extends 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 --- tools/testing/selftests/arm64/.gitignore | 1 + tools/testing/selftests/arm64/Makefile| 11 +++ .../testing/selftests/arm64/run_tags_test.sh | 12 tools/testing/selftests/arm64/tags_test.c | 29 +++ 4 files changed, 53 insertions(+) create mode 100644 tools/testing/selftests/arm64/.gitignore create mode 100644 tools/testing/selftests/arm64/Makefile create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh create mode 100644 tools/testing/selftests/arm64/tags_test.c diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore new file mode 100644 index ..e8fae8d61ed6 --- /dev/null +++ b/tools/testing/selftests/arm64/.gitignore @@ -0,0 +1 @@ +tags_test diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile new file mode 100644 index ..a61b2e743e99 --- /dev/null +++ b/tools/testing/selftests/arm64/Makefile @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0 + +# ARCH can be overridden by the user for cross compiling +ARCH ?= $(shell uname -m 2>/dev/null || echo not) + +ifneq (,$(filter $(ARCH),aarch64 arm64)) +TEST_GEN_PROGS := tags_test +TEST_PROGS := run_tags_test.sh +endif + +include ../lib.mk diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh new file mode 100755 index ..745f11379930 --- /dev/null +++ b/tools/testing/selftests/arm64/run_tags_test.sh @@ -0,0 +1,12 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +echo "" +echo "running tags test" +echo "" +./tags_test +if [ $? -ne 0 ]; then + echo "[FAIL]" +else + echo "[PASS]" +fi diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c new file mode 100644 index ..22a1b266e373 --- /dev/null +++ b/tools/testing/selftests/arm64/tags_test.c @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include +#include +#include + +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56) +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \ + SHIFT_TAG(tag)) + +int main(void) +{ + static int tbi_enabled = 0; + struct utsname *ptr, *tagged_ptr; + int err; + + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) + tbi_enabled = 1; + ptr = (struct utsname *)malloc(sizeof(*ptr)); + if (tbi_enabled) + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42); + err = uname(tagged_ptr); + free(ptr); + + return err; +} -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/9] drm: meson: drv: use macro when initializing vpu
This patch add new macro which is used to set WRARB/RDARB mode of the VPU. Signed-off-by: Julien Masson --- drivers/gpu/drm/meson/meson_drv.c | 26 + drivers/gpu/drm/meson/meson_registers.h | 1 + 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 2310c96fff46..50096697adc3 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -149,10 +149,28 @@ static struct regmap_config meson_regmap_config = { static void meson_vpu_init(struct meson_drm *priv) { - writel_relaxed(0x21, priv->io_base + _REG(VPU_RDARB_MODE_L1C1)); - writel_relaxed(0x1, priv->io_base + _REG(VPU_RDARB_MODE_L1C2)); - writel_relaxed(0x90, priv->io_base + _REG(VPU_RDARB_MODE_L2C1)); - writel_relaxed(0x2, priv->io_base + _REG(VPU_WRARB_MODE_L2C1)); + u32 value; + + /* +* Slave dc0 and dc5 connected to master port 1. +* By default other slaves are connected to master port 0. +*/ + value = VPU_RDARB_SLAVE_TO_MASTER_PORT(0, 1) | + VPU_RDARB_SLAVE_TO_MASTER_PORT(5, 1); + writel_relaxed(value, priv->io_base + _REG(VPU_RDARB_MODE_L1C1)); + + /* Slave dc0 connected to master port 1 */ + value = VPU_RDARB_SLAVE_TO_MASTER_PORT(0, 1); + writel_relaxed(value, priv->io_base + _REG(VPU_RDARB_MODE_L1C2)); + + /* Slave dc4 and dc7 connected to master port 1 */ + value = VPU_RDARB_SLAVE_TO_MASTER_PORT(4, 1) | + VPU_RDARB_SLAVE_TO_MASTER_PORT(7, 1); + writel_relaxed(value, priv->io_base + _REG(VPU_RDARB_MODE_L2C1)); + + /* Slave dc1 connected to master port 1 */ + value = VPU_RDARB_SLAVE_TO_MASTER_PORT(1, 1); + writel_relaxed(value, priv->io_base + _REG(VPU_WRARB_MODE_L2C1)); } static void meson_remove_framebuffers(void) diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h index 55f5fe21ff5e..a9db49e5bdd6 100644 --- a/drivers/gpu/drm/meson/meson_registers.h +++ b/drivers/gpu/drm/meson/meson_registers.h @@ -1496,6 +1496,7 @@ #define VPU_RDARB_MODE_L1C2 0x2799 #define VPU_RDARB_MODE_L2C1 0x279d #define VPU_WRARB_MODE_L2C1 0x27a2 +#defineVPU_RDARB_SLAVE_TO_MASTER_PORT(dc, port) (port << (16 + dc)) /* osd super scale */ #define OSDSR_HV_SIZEIN 0x3130 -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/7] lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer
From: Alastair D'Silva This patch removes the hardcoded row limits and allows for other lengths. These lengths must still be a multiple of groupsize. This allows structs that are not 16/32 bytes to display on a single line. This patch also expands the self-tests to test row sizes up to 64 bytes (though they can now be arbitrarily long). Signed-off-by: Alastair D'Silva --- lib/hexdump.c | 49 +-- lib/test_hexdump.c | 52 ++ 2 files changed, 76 insertions(+), 25 deletions(-) diff --git a/lib/hexdump.c b/lib/hexdump.c index 81b70ed37209..870c8cbff1e1 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -12,6 +12,7 @@ #include #include #include +#include #include const char hex_asc[] = "0123456789abcdef"; @@ -80,14 +81,15 @@ EXPORT_SYMBOL(bin2hex); * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory * @buf: data blob to dump * @len: number of bytes in the @buf - * @rowsize: number of bytes to print per line; must be 16 or 32 + * @rowsize: number of bytes to print per line; must be a multiple of groupsize * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL * @ascii: include ASCII after the hex output * - * hex_dump_to_buffer() works on one "line" of output at a time, i.e., - * 16 or 32 bytes of input data converted to hex + ASCII output. + * hex_dump_to_buffer() works on one "line" of output at a time, converting + * bytes of input to hexadecimal (and optionally printable ASCII) + * until bytes have been emitted. * * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data * to a hex + ASCII dump at the supplied memory location. @@ -116,16 +118,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, int ascii_column; int ret; - if (rowsize != 16 && rowsize != 32) - rowsize = 16; - - if (len > rowsize) /* limit to one line at a time */ - len = rowsize; if (!is_power_of_2(groupsize) || groupsize > 8) groupsize = 1; if ((len % groupsize) != 0) /* no mixed size output */ groupsize = 1; + if (rowsize % groupsize) + rowsize -= rowsize % groupsize; + + if (len > rowsize) /* limit to one line at a time */ + len = rowsize; + ngroups = len / groupsize; ascii_column = rowsize * 2 + rowsize / groupsize + 1; @@ -216,7 +219,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * caller supplies trailing spaces for alignment if desired * @prefix_type: controls whether prefix of an offset, address, or none * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) - * @rowsize: number of bytes to print per line; must be 16 or 32 + * @rowsize: number of bytes to print per line; must be a multiple of groupsize * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) * @buf: data blob to dump * @len: number of bytes in the @buf @@ -226,10 +229,9 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * to the kernel log at the specified kernel log level, with an optional * leading prefix. * - * print_hex_dump() works on one "line" of output at a time, i.e., - * 16 or 32 bytes of input data converted to hex + ASCII output. * print_hex_dump() iterates over the entire input @buf, breaking it into - * "line size" chunks to format and print. + * lines of rowsize/groupsize groups of input data converted to hex + + * (optionally) ASCII output. * * E.g.: * print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS, @@ -246,17 +248,30 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, { const u8 *ptr = buf; int i, linelen, remaining = len; - unsigned char linebuf[32 * 3 + 2 + 32 + 1]; + unsigned char *linebuf; + unsigned int linebuf_len; - if (rowsize != 16 && rowsize != 32) - rowsize = 16; + if (rowsize % groupsize) + rowsize -= rowsize % groupsize; + + /* +* Worst case line length: +* 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL +*/ + linebuf_len = rowsize * 3 + 2 + rowsize + 1; + linebuf = kzalloc(linebuf_len, GFP_KERNEL); + if (!linebuf) { + printk("%s%shexdump: Could not alloc %u bytes for buffer\n", + level, prefix_str, linebuf_len); + return; + } for (i = 0; i < len; i += rowsize) { linelen = min(remaining, rowsize); remaining -= rowsize; hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, - linebuf, sizeof(linebuf), ascii); +
[PATCH v4 hmm 05/12] mm/hmm: Remove duplicate condition test before wait_event_timeout
From: Jason Gunthorpe The wait_event_timeout macro already tests the condition as its first action, so there is no reason to open code another version of this, all that does is skip the might_sleep() debugging in common cases, which is not helpful. Further, based on prior patches, we can now simplify the required condition test: - If range is valid memory then so is range->hmm - If hmm_release() has run then range->valid is set to false at the same time as dead, so no reason to check both. - A valid hmm has a valid hmm->mm. Allowing the return value of wait_event_timeout() (along with its internal barriers) to compute the result of the function. Signed-off-by: Jason Gunthorpe Reviewed-by: Ralph Campbell Reviewed-by: John Hubbard Reviewed-by: Ira Weiny Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v3 - Simplify the wait_event_timeout to not check valid --- include/linux/hmm.h | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/include/linux/hmm.h b/include/linux/hmm.h index 1d97b6d62c5bcf..26e7c477490c4e 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -209,17 +209,8 @@ static inline unsigned long hmm_range_page_size(const struct hmm_range *range) static inline bool hmm_range_wait_until_valid(struct hmm_range *range, unsigned long timeout) { - /* Check if mm is dead ? */ - if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) { - range->valid = false; - return false; - } - if (range->valid) - return true; - wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead, - msecs_to_jiffies(timeout)); - /* Return current valid status just in case we get lucky */ - return range->valid; + return wait_event_timeout(range->hmm->wq, range->valid, + msecs_to_jiffies(timeout)) != 0; } /* -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 0/7] Hexdump Enhancements
From: Alastair D'Silva Apologies for the large CC list, it's a heads up for those responsible for subsystems where a prototype change in generic code causes a change in those subsystems. This series enhances hexdump. These improve the readability of the dumped data in certain situations (eg. wide terminals are available, many lines of empty bytes exist, etc). The default behaviour of hexdump is unchanged, however, the prototype for hex_dump_to_buffer() has changed, and print_hex_dump() has been renamed to print_hex_dump_ext(), with a wrapper replacing it for compatibility with existing code, which would have been too invasive to change. Hexdump selftests have be run & confirmed passed. Changelog: V4: - Add missing header (linux/bits.h) - Fix comment formatting - Create hex_dump_to_buffer_ext & make hex_dump_to_buffer a wrapper V3: - Fix inline documention - use BIT macros - use u32 rather than u64 for flags V2: - Fix failing selftests - Fix precedence bug in 'Replace ascii bool in hex_dump_to_buffer...' - Remove hardcoded new lengths & instead relax the checks in hex_dump_to_buffer, allocating the buffer from the heap instead of the stack. - Replace the skipping of lines of 0x00/0xff with skipping lines of repeated characters, announcing what has been skipped. - Add spaces as an optional N-group separator - Allow byte ordering to be maintained when HEXDUMP_RETAIN_BYTE_ORDERING is set. - Updated selftests to cover 'Relax rowsize checks' & 'Optionally retain byte ordering' Alastair D'Silva (7): lib/hexdump.c: Fix selftests lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer lib/hexdump.c: Optionally suppress lines of repeated bytes lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags lib/hexdump.c: Allow multiple groups to be separated by lines '|' lib/hexdump.c: Allow multiple groups to be separated by spaces lib/hexdump.c: Optionally retain byte ordering drivers/gpu/drm/i915/intel_engine_cs.c| 5 +- drivers/isdn/hardware/mISDN/mISDNisar.c | 10 +- drivers/mailbox/mailbox-test.c| 8 +- drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- .../net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +- drivers/net/wireless/ath/ath10k/debug.c | 7 +- .../net/wireless/intel/iwlegacy/3945-mac.c| 4 +- drivers/platform/chrome/wilco_ec/debugfs.c| 10 +- drivers/scsi/scsi_logging.c | 8 +- drivers/staging/fbtft/fbtft-core.c| 2 +- fs/seq_file.c | 6 +- include/linux/printk.h| 75 - lib/hexdump.c | 267 +++--- lib/test_hexdump.c| 154 +++--- 14 files changed, 438 insertions(+), 122 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 06/15] mm: untag user pointers in get_vaddr_frames
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. get_vaddr_frames uses provided user pointers for vma lookups, which can only by done with untagged pointers. Instead of locating and changing all callers of this function, perform untagging in it. Reviewed-by: Khalid Aziz Reviewed-by: Vincenzo Frascino Acked-by: Catalin Marinas Reviewed-by: Kees Cook Signed-off-by: Andrey Konovalov --- mm/frame_vector.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/frame_vector.c b/mm/frame_vector.c index c64dca6e27c2..c431ca81dad5 100644 --- a/mm/frame_vector.c +++ b/mm/frame_vector.c @@ -46,6 +46,8 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, if (WARN_ON_ONCE(nr_frames > vec->nr_allocated)) nr_frames = vec->nr_allocated; + start = untagged_addr(start); + down_read(&mm->mmap_sem); locked = 1; vma = find_vma_intersection(mm, start, start + 1); -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 04/12] mm/hmm: Simplify hmm_get_or_create and make it reliable
From: Jason Gunthorpe As coded this function can false-fail in various racy situations. Make it reliable and simpler by running under the write side of the mmap_sem and avoiding the false-failing compare/exchange pattern. Due to the mmap_sem this no longer has to avoid racing with a 2nd parallel hmm_get_or_create(). Unfortunately this still has to use the page_table_lock as the non-sleeping lock protecting mm->hmm, since the contexts where we free the hmm are incompatible with mmap_sem. Signed-off-by: Jason Gunthorpe Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Reviewed-by: Ira Weiny Tested-by: Philip Yang --- v2: - Fix error unwind of mmgrab (Jerome) - Use hmm local instead of 2nd container_of (Jerome) v3: - Can't use mmap_sem in the SRCU callback, keep using the page_table_lock (Philip) v4: - Put the mm->hmm = NULL in the kref release, reduce LOC in hmm_get_or_create() (Christoph) --- mm/hmm.c | 77 ++-- 1 file changed, 30 insertions(+), 47 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 080b17a2e87e2d..0423f4ca3a7e09 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -31,16 +31,6 @@ #if IS_ENABLED(CONFIG_HMM_MIRROR) static const struct mmu_notifier_ops hmm_mmu_notifier_ops; -static inline struct hmm *mm_get_hmm(struct mm_struct *mm) -{ - struct hmm *hmm = READ_ONCE(mm->hmm); - - if (hmm && kref_get_unless_zero(&hmm->kref)) - return hmm; - - return NULL; -} - /** * hmm_get_or_create - register HMM against an mm (HMM internal) * @@ -55,11 +45,16 @@ static inline struct hmm *mm_get_hmm(struct mm_struct *mm) */ static struct hmm *hmm_get_or_create(struct mm_struct *mm) { - struct hmm *hmm = mm_get_hmm(mm); - bool cleanup = false; + struct hmm *hmm; + + lockdep_assert_held_exclusive(&mm->mmap_sem); - if (hmm) - return hmm; + /* Abuse the page_table_lock to also protect mm->hmm. */ + spin_lock(&mm->page_table_lock); + hmm = mm->hmm; + if (mm->hmm && kref_get_unless_zero(&mm->hmm->kref)) + goto out_unlock; + spin_unlock(&mm->page_table_lock); hmm = kmalloc(sizeof(*hmm), GFP_KERNEL); if (!hmm) @@ -74,57 +69,45 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm) hmm->notifiers = 0; hmm->dead = false; hmm->mm = mm; - mmgrab(hmm->mm); - spin_lock(&mm->page_table_lock); - if (!mm->hmm) - mm->hmm = hmm; - else - cleanup = true; - spin_unlock(&mm->page_table_lock); + hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops; + if (__mmu_notifier_register(&hmm->mmu_notifier, mm)) { + kfree(hmm); + return NULL; + } - if (cleanup) - goto error; + mmgrab(hmm->mm); /* -* We should only get here if hold the mmap_sem in write mode ie on -* registration of first mirror through hmm_mirror_register() +* We hold the exclusive mmap_sem here so we know that mm->hmm is +* still NULL or 0 kref, and is safe to update. */ - hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops; - if (__mmu_notifier_register(&hmm->mmu_notifier, mm)) - goto error_mm; - - return hmm; - -error_mm: spin_lock(&mm->page_table_lock); - if (mm->hmm == hmm) - mm->hmm = NULL; + mm->hmm = hmm; + +out_unlock: spin_unlock(&mm->page_table_lock); -error: - mmdrop(hmm->mm); - kfree(hmm); - return NULL; + return hmm; } static void hmm_free_rcu(struct rcu_head *rcu) { - kfree(container_of(rcu, struct hmm, rcu)); + struct hmm *hmm = container_of(rcu, struct hmm, rcu); + + mmdrop(hmm->mm); + kfree(hmm); } static void hmm_free(struct kref *kref) { struct hmm *hmm = container_of(kref, struct hmm, kref); - struct mm_struct *mm = hmm->mm; - mmu_notifier_unregister_no_release(&hmm->mmu_notifier, mm); + spin_lock(&hmm->mm->page_table_lock); + if (hmm->mm->hmm == hmm) + hmm->mm->hmm = NULL; + spin_unlock(&hmm->mm->page_table_lock); - spin_lock(&mm->page_table_lock); - if (mm->hmm == hmm) - mm->hmm = NULL; - spin_unlock(&mm->page_table_lock); - - mmdrop(hmm->mm); + mmu_notifier_unregister_no_release(&hmm->mmu_notifier, hmm->mm); mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu); } -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/7] lib/hexdump.c: Fix selftests
From: Alastair D'Silva The overflow tests did not account for the situation where no overflow occurs and len < rowsize. This patch renames the cryptic variables and accounts for the above case. The selftests now pass. Signed-off-by: Alastair D'Silva --- lib/test_hexdump.c | 48 +++--- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c index 5144899d3c6b..bef97a964582 100644 --- a/lib/test_hexdump.c +++ b/lib/test_hexdump.c @@ -163,45 +163,53 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, { char test[TEST_HEXDUMP_BUF_SIZE]; char buf[TEST_HEXDUMP_BUF_SIZE]; - int rs = rowsize, gs = groupsize; - int ae, he, e, f, r; - bool a; + int ascii_len, hex_len, expected_len, fill_point, ngroups, rc; + bool match; total_tests++; memset(buf, FILL_CHAR, sizeof(buf)); - r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii); + rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen, + ascii); /* * Caller must provide the data length multiple of groupsize. The * calculations below are made with that assumption in mind. */ - ae = rs * 2 /* hex */ + rs / gs /* spaces */ + 1 /* space */ + len /* ascii */; - he = (gs * 2 /* hex */ + 1 /* space */) * len / gs - 1 /* no trailing space */; + ngroups = rowsize / groupsize; + hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups + - 1 /* no trailing space */; + ascii_len = hex_len + 2 /* space */ + len /* ascii */; + + if (len < rowsize) { + ngroups = len / groupsize; + hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups + - 1 /* no trailing space */; + } - if (ascii) - e = ae; - else - e = he; + expected_len = (ascii) ? ascii_len : hex_len; - f = min_t(int, e + 1, buflen); + fill_point = min_t(int, expected_len + 1, buflen); if (buflen) { - test_hexdump_prepare_test(len, rs, gs, test, sizeof(test), ascii); - test[f - 1] = '\0'; + test_hexdump_prepare_test(len, rowsize, groupsize, test, + sizeof(test), ascii); + test[fill_point - 1] = '\0'; } - memset(test + f, FILL_CHAR, sizeof(test) - f); + memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point); - a = r == e && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE); + match = rc == expected_len && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE); buf[sizeof(buf) - 1] = '\0'; - if (!a) { - pr_err("Len: %zu buflen: %zu strlen: %zu\n", - len, buflen, strnlen(buf, sizeof(buf))); - pr_err("Result: %d '%s'\n", r, buf); - pr_err("Expect: %d '%s'\n", e, test); + if (!match) { + pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: %zu strlen: %zu\n", + rowsize, groupsize, ascii, len, buflen, + strnlen(buf, sizeof(buf))); + pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf); + pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test); failed_tests++; + } } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/9] drm: meson: crtc: use proper macros instead of magic constants
This patch add new macros which describe couple bits field of the following registers: - VD1_BLEND_SRC_CTRL - VPP_SC_MISC Signed-off-by: Julien Masson --- drivers/gpu/drm/meson/meson_crtc.c | 17 +++-- drivers/gpu/drm/meson/meson_registers.h | 16 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c index aa8ea107524e..6f7d6d258615 100644 --- a/drivers/gpu/drm/meson/meson_crtc.c +++ b/drivers/gpu/drm/meson/meson_crtc.c @@ -267,11 +267,11 @@ static void meson_crtc_enable_vd1(struct meson_drm *priv) static void meson_g12a_crtc_enable_vd1(struct meson_drm *priv) { - writel_relaxed(((1 << 16) | /* post bld premult*/ - (1 << 8) | /* post src */ - (1 << 4) | /* pre bld premult*/ - (1 << 0)), - priv->io_base + _REG(VD1_BLEND_SRC_CTRL)); + writel_relaxed(VD_BLEND_PREBLD_SRC_VD1 | + VD_BLEND_PREBLD_PREMULT_EN | + VD_BLEND_POSTBLD_SRC_VD1 | + VD_BLEND_POSTBLD_PREMULT_EN, + priv->io_base + _REG(VD1_BLEND_SRC_CTRL)); } void meson_crtc_irq(struct meson_drm *priv) @@ -489,7 +489,12 @@ void meson_crtc_irq(struct meson_drm *priv) writel_relaxed(priv->viu.vd1_range_map_cr, priv->io_base + meson_crtc->viu_offset + _REG(VD1_IF0_RANGE_MAP_CR)); - writel_relaxed(0x78404, + writel_relaxed(VPP_VSC_BANK_LENGTH(4) | + VPP_HSC_BANK_LENGTH(4) | + VPP_SC_VD_EN_ENABLE | + VPP_SC_TOP_EN_ENABLE | + VPP_SC_HSC_EN_ENABLE | + VPP_SC_VSC_EN_ENABLE, priv->io_base + _REG(VPP_SC_MISC)); writel_relaxed(priv->viu.vpp_pic_in_height, priv->io_base + _REG(VPP_PIC_IN_HEIGHT)); diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h index c7dfbd7454e5..55f5fe21ff5e 100644 --- a/drivers/gpu/drm/meson/meson_registers.h +++ b/drivers/gpu/drm/meson/meson_registers.h @@ -370,6 +370,12 @@ #define VPP_HSC_REGION4_PHASE_SLOPE 0x1d17 #define VPP_HSC_PHASE_CTRL 0x1d18 #define VPP_SC_MISC 0x1d19 +#defineVPP_SC_VD_EN_ENABLE BIT(15) +#defineVPP_SC_TOP_EN_ENABLEBIT(16) +#defineVPP_SC_HSC_EN_ENABLEBIT(17) +#defineVPP_SC_VSC_EN_ENABLEBIT(18) +#defineVPP_VSC_BANK_LENGTH(length) (length & 0x7) +#defineVPP_HSC_BANK_LENGTH(length) ((length & 0x7) << 8) #define VPP_PREBLEND_VD1_H_START_END 0x1d1a #define VPP_PREBLEND_VD1_V_START_END 0x1d1b #define VPP_POSTBLEND_VD1_H_START_END 0x1d1c @@ -1638,6 +1644,16 @@ #define VPP_SLEEP_CTRL 0x1dfa #define VD1_BLEND_SRC_CTRL 0x1dfb #define VD2_BLEND_SRC_CTRL 0x1dfc +#defineVD_BLEND_PREBLD_SRC_VD1 (1 << 0) +#defineVD_BLEND_PREBLD_SRC_VD2 (2 << 0) +#defineVD_BLEND_PREBLD_SRC_OSD1(3 << 0) +#defineVD_BLEND_PREBLD_SRC_OSD2(4 << 0) +#defineVD_BLEND_PREBLD_PREMULT_EN BIT(4) +#defineVD_BLEND_POSTBLD_SRC_VD1(1 << 8) +#defineVD_BLEND_POSTBLD_SRC_VD2(2 << 8) +#defineVD_BLEND_POSTBLD_SRC_OSD1 (3 << 8) +#defineVD_BLEND_POSTBLD_SRC_OSD2 (4 << 8) +#defineVD_BLEND_POSTBLD_PREMULT_EN BIT(16) #define OSD1_BLEND_SRC_CTRL 0x1dfd #define OSD2_BLEND_SRC_CTRL 0x1dfe -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v18 08/15] userfaultfd: untag user pointers
On Mon, Jun 24, 2019 at 04:32:53PM +0200, Andrey Konovalov wrote: > This patch is a part of a series that extends kernel ABI to allow to pass > tagged user pointers (with the top byte set to something else other than > 0x00) as syscall arguments. > > userfaultfd code use provided user pointers for vma lookups, which can > only by done with untagged pointers. > > Untag user pointers in validate_range(). > > Reviewed-by: Vincenzo Frascino > Reviewed-by: Catalin Marinas > Reviewed-by: Kees Cook > Signed-off-by: Andrey Konovalov > --- > fs/userfaultfd.c | 22 -- > 1 file changed, 12 insertions(+), 10 deletions(-) Same here, it needs an ack from Al Viro. > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index ae0b8b5f69e6..c2be36a168ca 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct > userfaultfd_ctx *ctx, > } > > static __always_inline int validate_range(struct mm_struct *mm, > - __u64 start, __u64 len) > + __u64 *start, __u64 len) > { > __u64 task_size = mm->task_size; > > - if (start & ~PAGE_MASK) > + *start = untagged_addr(*start); > + > + if (*start & ~PAGE_MASK) > return -EINVAL; > if (len & ~PAGE_MASK) > return -EINVAL; > if (!len) > return -EINVAL; > - if (start < mmap_min_addr) > + if (*start < mmap_min_addr) > return -EINVAL; > - if (start >= task_size) > + if (*start >= task_size) > return -EINVAL; > - if (len > task_size - start) > + if (len > task_size - *start) > return -EINVAL; > return 0; > } > @@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx > *ctx, > goto out; > } > > - ret = validate_range(mm, uffdio_register.range.start, > + ret = validate_range(mm, &uffdio_register.range.start, >uffdio_register.range.len); > if (ret) > goto out; > @@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct > userfaultfd_ctx *ctx, > if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister))) > goto out; > > - ret = validate_range(mm, uffdio_unregister.start, > + ret = validate_range(mm, &uffdio_unregister.start, >uffdio_unregister.len); > if (ret) > goto out; > @@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx, > if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake))) > goto out; > > - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len); > + ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len); > if (ret) > goto out; > > @@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, > sizeof(uffdio_copy)-sizeof(__s64))) > goto out; > > - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len); > + ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len); > if (ret) > goto out; > /* > @@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx > *ctx, > sizeof(uffdio_zeropage)-sizeof(__s64))) > goto out; > > - ret = validate_range(ctx->mm, uffdio_zeropage.range.start, > + ret = validate_range(ctx->mm, &uffdio_zeropage.range.start, >uffdio_zeropage.range.len); > if (ret) > goto out; > -- > 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 01/12] mm/hmm: fix use after free with struct hmm in the mmu notifiers
From: Jason Gunthorpe mmu_notifier_unregister_no_release() is not a fence and the mmu_notifier system will continue to reference hmm->mn until the srcu grace period expires. Resulting in use after free races like this: CPU0 CPU1 __mmu_notifier_invalidate_range_start() srcu_read_lock hlist_for_each () // mn == hmm->mn hmm_mirror_unregister() hmm_put() hmm_free() mmu_notifier_unregister_no_release() hlist_del_init_rcu(hmm-mn->list) mn->ops->invalidate_range_start(mn, range); mm_get_hmm() mm->hmm = NULL; kfree(hmm) mutex_lock(&hmm->lock); Use SRCU to kfree the hmm memory so that the notifiers can rely on hmm existing. Get the now-safe hmm struct through container_of and directly check kref_get_unless_zero to lock it against free. Signed-off-by: Jason Gunthorpe Reviewed-by: Ira Weiny Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v2: - Spell 'free' properly (Jerome/Ralph) v3: - Have only one clearer comment about kref_get_unless_zero (John) --- include/linux/hmm.h | 1 + mm/hmm.c| 23 +-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/include/linux/hmm.h b/include/linux/hmm.h index 7007123842ba76..cb01cf1fa3c08b 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -93,6 +93,7 @@ struct hmm { struct mmu_notifier mmu_notifier; struct rw_semaphore mirrors_sem; wait_queue_head_t wq; + struct rcu_head rcu; longnotifiers; booldead; }; diff --git a/mm/hmm.c b/mm/hmm.c index 826816ab237799..f6956d78e3cb25 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -104,6 +104,11 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm) return NULL; } +static void hmm_free_rcu(struct rcu_head *rcu) +{ + kfree(container_of(rcu, struct hmm, rcu)); +} + static void hmm_free(struct kref *kref) { struct hmm *hmm = container_of(kref, struct hmm, kref); @@ -116,7 +121,7 @@ static void hmm_free(struct kref *kref) mm->hmm = NULL; spin_unlock(&mm->page_table_lock); - kfree(hmm); + mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu); } static inline void hmm_put(struct hmm *hmm) @@ -144,10 +149,14 @@ void hmm_mm_destroy(struct mm_struct *mm) static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm) { - struct hmm *hmm = mm_get_hmm(mm); + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier); struct hmm_mirror *mirror; struct hmm_range *range; + /* Bail out if hmm is in the process of being freed */ + if (!kref_get_unless_zero(&hmm->kref)) + return; + /* Report this HMM as dying. */ hmm->dead = true; @@ -185,13 +194,14 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm) static int hmm_invalidate_range_start(struct mmu_notifier *mn, const struct mmu_notifier_range *nrange) { - struct hmm *hmm = mm_get_hmm(nrange->mm); + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier); struct hmm_mirror *mirror; struct hmm_update update; struct hmm_range *range; int ret = 0; - VM_BUG_ON(!hmm); + if (!kref_get_unless_zero(&hmm->kref)) + return 0; update.start = nrange->start; update.end = nrange->end; @@ -236,9 +246,10 @@ static int hmm_invalidate_range_start(struct mmu_notifier *mn, static void hmm_invalidate_range_end(struct mmu_notifier *mn, const struct mmu_notifier_range *nrange) { - struct hmm *hmm = mm_get_hmm(nrange->mm); + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier); - VM_BUG_ON(!hmm); + if (!kref_get_unless_zero(&hmm->kref)) + return; mutex_lock(&hmm->lock); hmm->notifiers--; -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 02/12] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
From: Jason Gunthorpe Ralph observes that hmm_range_register() can only be called by a driver while a mirror is registered. Make this clear in the API by passing in the mirror structure as a parameter. This also simplifies understanding the lifetime model for struct hmm, as the hmm pointer must be valid as part of a registered mirror so all we need in hmm_register_range() is a simple kref_get. Suggested-by: Ralph Campbell Signed-off-by: Jason Gunthorpe Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Reviewed-by: Ira Weiny Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v2 - Include the oneline patch to nouveau_svm.c --- drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/hmm.h | 7 --- mm/hmm.c | 13 - 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 93ed43c413f0bb..8c92374afcf227 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify) range.values = nouveau_svm_pfn_values; range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT; again: - ret = hmm_vma_fault(&range, true); + ret = hmm_vma_fault(&svmm->mirror, &range, true); if (ret == 0) { mutex_lock(&svmm->mutex); if (!hmm_vma_range_done(&range)) { diff --git a/include/linux/hmm.h b/include/linux/hmm.h index cb01cf1fa3c08b..1fba6979adf460 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -496,7 +496,7 @@ static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror *mirror) * Please see Documentation/vm/hmm.rst for how to use the range API. */ int hmm_range_register(struct hmm_range *range, - struct mm_struct *mm, + struct hmm_mirror *mirror, unsigned long start, unsigned long end, unsigned page_shift); @@ -532,7 +532,8 @@ static inline bool hmm_vma_range_done(struct hmm_range *range) } /* This is a temporary helper to avoid merge conflict between trees. */ -static inline int hmm_vma_fault(struct hmm_range *range, bool block) +static inline int hmm_vma_fault(struct hmm_mirror *mirror, + struct hmm_range *range, bool block) { long ret; @@ -545,7 +546,7 @@ static inline int hmm_vma_fault(struct hmm_range *range, bool block) range->default_flags = 0; range->pfn_flags_mask = -1UL; - ret = hmm_range_register(range, range->vma->vm_mm, + ret = hmm_range_register(range, mirror, range->start, range->end, PAGE_SHIFT); if (ret) diff --git a/mm/hmm.c b/mm/hmm.c index f6956d78e3cb25..22a97ada108b4e 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -914,13 +914,13 @@ static void hmm_pfns_clear(struct hmm_range *range, * Track updates to the CPU page table see include/linux/hmm.h */ int hmm_range_register(struct hmm_range *range, - struct mm_struct *mm, + struct hmm_mirror *mirror, unsigned long start, unsigned long end, unsigned page_shift) { unsigned long mask = ((1UL << page_shift) - 1UL); - struct hmm *hmm; + struct hmm *hmm = mirror->hmm; range->valid = false; range->hmm = NULL; @@ -934,20 +934,15 @@ int hmm_range_register(struct hmm_range *range, range->start = start; range->end = end; - hmm = hmm_get_or_create(mm); - if (!hmm) - return -EFAULT; - /* Check if hmm_mm_destroy() was call. */ - if (hmm->mm == NULL || hmm->dead) { - hmm_put(hmm); + if (hmm->mm == NULL || hmm->dead) return -EFAULT; - } /* Initialize range to track CPU page table updates. */ mutex_lock(&hmm->lock); range->hmm = hmm; + kref_get(&hmm->kref); list_add_rcu(&range->list, &hmm->ranges); /* -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 09/15] drm/amdgpu: untag user pointers
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. In amdgpu_gem_userptr_ioctl() and amdgpu_amdkfd_gpuvm.c/init_user_pages() an MMU notifier is set up with a (tagged) userspace pointer. The untagged address should be used so that MMU notifiers for the untagged address get correctly matched up with the right BO. This patch untag user pointers in amdgpu_gem_userptr_ioctl() for the GEM case and in amdgpu_amdkfd_gpuvm_ alloc_memory_of_gpu() for the KFD case. This also makes sure that an untagged pointer is passed to amdgpu_ttm_tt_get_user_pages(), which uses it for vma lookups. Suggested-by: Felix Kuehling Acked-by: Felix Kuehling Signed-off-by: Andrey Konovalov --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index a6e5184d436c..5d476e9bbc43 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1108,7 +1108,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu( alloc_flags = 0; if (!offset || !*offset) return -EINVAL; - user_addr = *offset; + user_addr = untagged_addr(*offset); } else if (flags & ALLOC_MEM_FLAGS_DOORBELL) { domain = AMDGPU_GEM_DOMAIN_GTT; alloc_domain = AMDGPU_GEM_DOMAIN_CPU; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index d4fcf5475464..e91df1407618 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -287,6 +287,8 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data, uint32_t handle; int r; + args->addr = untagged_addr(args->addr); + if (offset_in_page(args->addr | args->size)) return -EINVAL; -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can only by done with untagged pointers. Untag user pointers in this function. Signed-off-by: Andrey Konovalov --- drivers/infiniband/hw/mlx4/mr.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c index 355205a28544..13d9f917f249 100644 --- a/drivers/infiniband/hw/mlx4/mr.c +++ b/drivers/infiniband/hw/mlx4/mr.c @@ -378,6 +378,7 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start, * again */ if (!ib_access_writable(access_flags)) { + unsigned long untagged_start = untagged_addr(start); struct vm_area_struct *vma; down_read(¤t->mm->mmap_sem); @@ -386,9 +387,9 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start, * cover the memory, but for now it requires a single vma to * entirely cover the MR to support RO mappings. */ - vma = find_vma(current->mm, start); - if (vma && vma->vm_end >= start + length && - vma->vm_start <= start) { + vma = find_vma(current->mm, untagged_start); + if (vma && vma->vm_end >= untagged_start + length && + vma->vm_start <= untagged_start) { if (vma->vm_flags & VM_WRITE) access_flags |= IB_ACCESS_LOCAL_WRITE; } else { -- 2.22.0.410.gd8fdbe21b5-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 07/12] mm/hmm: Hold on to the mmget for the lifetime of the range
From: Jason Gunthorpe Range functions like hmm_range_snapshot() and hmm_range_fault() call find_vma, which requires hodling the mmget() and the mmap_sem for the mm. Make this simpler for the callers by holding the mmget() inside the range for the lifetime of the range. Other functions that accept a range should only be called if the range is registered. This has the side effect of directly preventing hmm_release() from happening while a range is registered. That means range->dead cannot be false during the lifetime of the range, so remove dead and hmm_mirror_mm_is_alive() entirely. Signed-off-by: Jason Gunthorpe Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v2: - Use Jerome's idea of just holding the mmget() for the range lifetime, rework the patch to use that as as simplification to remove dead in one step v3: - Use list_del_careful (Christoph) --- include/linux/hmm.h | 26 -- mm/hmm.c| 32 +++- 2 files changed, 11 insertions(+), 47 deletions(-) diff --git a/include/linux/hmm.h b/include/linux/hmm.h index 26e7c477490c4e..bf013e96525771 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -82,7 +82,6 @@ * @mirrors_sem: read/write semaphore protecting the mirrors list * @wq: wait queue for user waiting on a range invalidation * @notifiers: count of active mmu notifiers - * @dead: is the mm dead ? */ struct hmm { struct mm_struct*mm; @@ -95,7 +94,6 @@ struct hmm { wait_queue_head_t wq; struct rcu_head rcu; longnotifiers; - booldead; }; /* @@ -459,30 +457,6 @@ struct hmm_mirror { int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm); void hmm_mirror_unregister(struct hmm_mirror *mirror); -/* - * hmm_mirror_mm_is_alive() - test if mm is still alive - * @mirror: the HMM mm mirror for which we want to lock the mmap_sem - * Return: false if the mm is dead, true otherwise - * - * This is an optimization, it will not always accurately return false if the - * mm is dead; i.e., there can be false negatives (process is being killed but - * HMM is not yet informed of that). It is only intended to be used to optimize - * out cases where the driver is about to do something time consuming and it - * would be better to skip it if the mm is dead. - */ -static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror *mirror) -{ - struct mm_struct *mm; - - if (!mirror || !mirror->hmm) - return false; - mm = READ_ONCE(mirror->hmm->mm); - if (mirror->hmm->dead || !mm) - return false; - - return true; -} - /* * Please see Documentation/vm/hmm.rst for how to use the range API. */ diff --git a/mm/hmm.c b/mm/hmm.c index 73c8af4827fe87..1eddda45cefae7 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -67,7 +67,6 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm) mutex_init(&hmm->lock); kref_init(&hmm->kref); hmm->notifiers = 0; - hmm->dead = false; hmm->mm = mm; hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops; @@ -120,21 +119,16 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm) { struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier); struct hmm_mirror *mirror; - struct hmm_range *range; /* Bail out if hmm is in the process of being freed */ if (!kref_get_unless_zero(&hmm->kref)) return; - /* Report this HMM as dying. */ - hmm->dead = true; - - /* Wake-up everyone waiting on any range. */ - mutex_lock(&hmm->lock); - list_for_each_entry(range, &hmm->ranges, list) - range->valid = false; - wake_up_all(&hmm->wq); - mutex_unlock(&hmm->lock); + /* +* Since hmm_range_register() holds the mmget() lock hmm_release() is +* prevented as long as a range exists. +*/ + WARN_ON(!list_empty_careful(&hmm->ranges)); down_write(&hmm->mirrors_sem); mirror = list_first_entry_or_null(&hmm->mirrors, struct hmm_mirror, @@ -903,8 +897,8 @@ int hmm_range_register(struct hmm_range *range, range->start = start; range->end = end; - /* Check if hmm_mm_destroy() was call. */ - if (hmm->mm == NULL || hmm->dead) + /* Prevent hmm_release() from running while the range is valid */ + if (!mmget_not_zero(hmm->mm)) return -EFAULT; /* Initialize range to track CPU page table updates. */ @@ -942,11 +936,12 @@ void hmm_range_unregister(struct hmm_range *range) return; mutex_lock(&hmm->lock); - list_del(&range->list); + list_del_init(&range->list); mutex_unlock(&hmm->lock); /* Drop reference taken by hmm_range_register() */ range->valid = false; +
[PATCH v4 hmm 03/12] mm/hmm: Hold a mmgrab from hmm to mm
From: Jason Gunthorpe So long as a struct hmm pointer exists, so should the struct mm it is linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it once the hmm refcount goes to zero. Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL mm->hmm delete the hmm_hmm_destroy(). Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: John Hubbard Reviewed-by: Ralph Campbell Reviewed-by: Ira Weiny Reviewed-by: Christoph Hellwig Tested-by: Philip Yang --- v2: - Fix error unwind paths in hmm_get_or_create (Jerome/Jason) --- include/linux/hmm.h | 3 --- kernel/fork.c | 1 - mm/hmm.c| 22 -- 3 files changed, 4 insertions(+), 22 deletions(-) diff --git a/include/linux/hmm.h b/include/linux/hmm.h index 1fba6979adf460..1d97b6d62c5bcf 100644 --- a/include/linux/hmm.h +++ b/include/linux/hmm.h @@ -577,14 +577,11 @@ static inline int hmm_vma_fault(struct hmm_mirror *mirror, } /* Below are for HMM internal use only! Not to be used by device driver! */ -void hmm_mm_destroy(struct mm_struct *mm); - static inline void hmm_mm_init(struct mm_struct *mm) { mm->hmm = NULL; } #else /* IS_ENABLED(CONFIG_HMM_MIRROR) */ -static inline void hmm_mm_destroy(struct mm_struct *mm) {} static inline void hmm_mm_init(struct mm_struct *mm) {} #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */ diff --git a/kernel/fork.c b/kernel/fork.c index 75675b9bf6dfd3..c704c3cedee78d 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm) WARN_ON_ONCE(mm == current->active_mm); mm_free_pgd(mm); destroy_context(mm); - hmm_mm_destroy(mm); mmu_notifier_mm_destroy(mm); check_mm(mm); put_user_ns(mm->user_ns); diff --git a/mm/hmm.c b/mm/hmm.c index 22a97ada108b4e..080b17a2e87e2d 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -73,6 +74,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm) hmm->notifiers = 0; hmm->dead = false; hmm->mm = mm; + mmgrab(hmm->mm); spin_lock(&mm->page_table_lock); if (!mm->hmm) @@ -100,6 +102,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm) mm->hmm = NULL; spin_unlock(&mm->page_table_lock); error: + mmdrop(hmm->mm); kfree(hmm); return NULL; } @@ -121,6 +124,7 @@ static void hmm_free(struct kref *kref) mm->hmm = NULL; spin_unlock(&mm->page_table_lock); + mmdrop(hmm->mm); mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu); } @@ -129,24 +133,6 @@ static inline void hmm_put(struct hmm *hmm) kref_put(&hmm->kref, hmm_free); } -void hmm_mm_destroy(struct mm_struct *mm) -{ - struct hmm *hmm; - - spin_lock(&mm->page_table_lock); - hmm = mm_get_hmm(mm); - mm->hmm = NULL; - if (hmm) { - hmm->mm = NULL; - hmm->dead = true; - spin_unlock(&mm->page_table_lock); - hmm_put(hmm); - return; - } - - spin_unlock(&mm->page_table_lock); -} - static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm) { struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier); -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 hmm 00/12]
From: Jason Gunthorpe 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(). The use of 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. I'll apply this in the next few days - the only patch that doesn't have enough Reviewed-bys is 'mm/hmm: Remove confusing comment and logic from hmm_release', which had alot of questions, I still think it is good. If people really don't like it I'll drop it. Thanks to everyone who took time to look at this! Jason Gunthorpe (12): mm/hmm: fix use after free with struct hmm in the mmu notifiers mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register mm/hmm: Hold a mmgrab from hmm to mm mm/hmm: Simplify hmm_get_or_create and make it reliable mm/hmm: Remove duplicate condition test before wait_event_timeout mm/hmm: Do not use list*_rcu() for hmm->ranges mm/hmm: Hold on to the mmget for the lifetime of the range mm/hmm: Use lockdep instead of comments mm/hmm: Remove racy protection against double-unregistration mm/hmm: Poison hmm_range during unregister mm/hmm: Remove confusing comment and logic from hmm_release mm/hmm: Fix error flows in hmm_invalidate_range_start drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/hmm.h | 52 + kernel/fork.c | 1 - mm/hmm.c | 275 -- 4 files changed, 130 insertions(+), 200 deletions(-) -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 6/7] lib/hexdump.c: Allow multiple groups to be separated by spaces
From: Alastair D'Silva Similar to the previous patch, this patch separates groups by 2 spaces for the hex fields, and 1 space for the ASCII field. eg. buf:: 454d414e 43415053 4e495f45 00584544 NAMESPAC E_INDEX. buf:0010: 0002 Signed-off-by: Alastair D'Silva --- include/linux/printk.h | 3 ++ lib/hexdump.c | 65 +++--- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index ae80d7af82ab..1d082291facf 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -488,6 +488,9 @@ enum { #define HEXDUMP_2_GRP_LINESBIT(2) #define HEXDUMP_4_GRP_LINESBIT(3) #define HEXDUMP_8_GRP_LINESBIT(4) +#define HEXDUMP_2_GRP_SPACES BIT(5) +#define HEXDUMP_4_GRP_SPACES BIT(6) +#define HEXDUMP_8_GRP_SPACES BIT(7) extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, diff --git a/lib/hexdump.c b/lib/hexdump.c index 4dcf759fe048..e09e3cf8e595 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -91,9 +91,37 @@ static const char *group_separator(int group, u64 flags) if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2)) return "|"; + if ((flags & HEXDUMP_8_GRP_SPACES) && !((group) % 8)) + return " "; + + if ((flags & HEXDUMP_4_GRP_SPACES) && !((group) % 4)) + return " "; + + if ((flags & HEXDUMP_2_GRP_SPACES) && !((group) % 2)) + return " "; + return " "; } +static void separator_parameters(u64 flags, int groupsize, int *sep_chars, +char *sep) +{ + if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_2_GRP_SPACES)) + *sep_chars = groupsize * 2; + if (flags & (HEXDUMP_4_GRP_LINES | HEXDUMP_4_GRP_SPACES)) + *sep_chars = groupsize * 4; + if (flags & (HEXDUMP_8_GRP_LINES | HEXDUMP_8_GRP_SPACES)) + *sep_chars = groupsize * 8; + + if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_4_GRP_LINES | + HEXDUMP_8_GRP_LINES)) + *sep = '|'; + + if (flags & (HEXDUMP_2_GRP_SPACES | HEXDUMP_4_GRP_SPACES | + HEXDUMP_8_GRP_SPACES)) + *sep = ' '; +} + /** * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory * @buf: data blob to dump @@ -107,6 +135,9 @@ static const char *group_separator(int group, u64 flags) * HEXDUMP_2_GRP_LINES:insert a '|' after every 2 groups * HEXDUMP_4_GRP_LINES:insert a '|' after every 4 groups * HEXDUMP_8_GRP_LINES:insert a '|' after every 8 groups + * HEXDUMP_2_GRP_SPACES: insert a ' ' after every 2 groups + * HEXDUMP_4_GRP_SPACES: insert a ' ' after every 4 groups + * HEXDUMP_8_GRP_SPACES: insert a ' ' after every 8 groups * * hex_dump_to_buffer() works on one "line" of output at a time, converting * bytes of input to hexadecimal (and optionally printable ASCII) @@ -139,7 +170,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, int j, lx = 0; int ascii_column; int ret; - int line_chars = 0; + int sep_chars = 0; + char sep = 0; if (!is_power_of_2(groupsize) || groupsize > 8) groupsize = 1; @@ -153,8 +185,14 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, len = rowsize; ngroups = len / groupsize; + ascii_column = rowsize * 2 + rowsize / groupsize + 1; + // space separators use 2 spaces in the hex output + separator_parameters(flags, groupsize, &sep_chars, &sep); + if (sep == ' ') + ascii_column += rowsize / sep_chars; + if (!linebuflen) goto overflow1; @@ -222,24 +260,17 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize, linebuf[lx++] = ' '; } - if (flags & HEXDUMP_2_GRP_LINES) - line_chars = groupsize * 2; - if (flags & HEXDUMP_4_GRP_LINES) - line_chars = groupsize * 4; - if (flags & HEXDUMP_8_GRP_LINES) - line_chars = groupsize * 8; - for (j = 0; j < len; j++) { if (linebuflen < lx + 2) goto overflow2; ch = ptr[j]; linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.'; - if (line_chars && ((j + 1) < len) && - ((j + 1) % line_chars == 0)) { + if (sep_chars && ((j + 1) < len) && + ((j + 1) % sep_chars == 0)) { if (linebuflen < lx + 2)
[PATCH 2/2] backlight: arcxcnn: add "arctic" vendor prefix
The original patch adding this driver and DT bindings improperly used "arc" as the vendor-prefix. This adds "arctic" which is the proper prefix and retains "arc" to allow existing users of the "arc" prefix to update to new kernels. There is at least one (Samsung Chromebook Plus) Signed-off-by: Brian Dodge --- drivers/video/backlight/arcxcnn_bl.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/video/backlight/arcxcnn_bl.c b/drivers/video/backlight/arcxcnn_bl.c index 7b1c0a0..14c67f2 100644 --- a/drivers/video/backlight/arcxcnn_bl.c +++ b/drivers/video/backlight/arcxcnn_bl.c @@ -1,9 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices + * Backlight driver for pSemi (formerly ArcticSand) ARC_X_C_0N_0N Devices * - * Copyright 2016 ArcticSand, Inc. - * Author : Brian Dodge + * Copyright 2016-2019 pSemi, Inc. + * Author : Brian Dodge */ #include @@ -191,27 +191,40 @@ static void arcxcnn_parse_dt(struct arcxcnn *lp) if (ret == 0) lp->pdata->initial_brightness = prog_val; - ret = of_property_read_u32(node, "arc,led-config-0", &prog_val); + ret = of_property_read_u32(node, "arctic,led-config-0", &prog_val); + if (ret) + ret = of_property_read_u32(node, "arc,led-config-0", &prog_val); if (ret == 0) lp->pdata->led_config_0 = (u8)prog_val; - ret = of_property_read_u32(node, "arc,led-config-1", &prog_val); + ret = of_property_read_u32(node, "arctic,led-config-1", &prog_val); + if (ret) + ret = of_property_read_u32(node, "arc,led-config-1", &prog_val); if (ret == 0) lp->pdata->led_config_1 = (u8)prog_val; - ret = of_property_read_u32(node, "arc,dim-freq", &prog_val); + ret = of_property_read_u32(node, "arctic,dim-freq", &prog_val); + if (ret) + ret = of_property_read_u32(node, "arc,dim-freq", &prog_val); if (ret == 0) lp->pdata->dim_freq = (u8)prog_val; - ret = of_property_read_u32(node, "arc,comp-config", &prog_val); + ret = of_property_read_u32(node, "arctic,comp-config", &prog_val); + if (ret) + ret = of_property_read_u32(node, "arc,comp-config", &prog_val); if (ret == 0) lp->pdata->comp_config = (u8)prog_val; - ret = of_property_read_u32(node, "arc,filter-config", &prog_val); + ret = of_property_read_u32(node, "arctic,filter-config", &prog_val); + if (ret) + ret = of_property_read_u32(node, + "arc,filter-config", &prog_val); if (ret == 0) lp->pdata->filter_config = (u8)prog_val; - ret = of_property_read_u32(node, "arc,trim-config", &prog_val); + ret = of_property_read_u32(node, "arctic,trim-config", &prog_val); + if (ret) + ret = of_property_read_u32(node, "arc,trim-config", &prog_val); if (ret == 0) lp->pdata->trim_config = (u8)prog_val; @@ -381,6 +394,8 @@ static int arcxcnn_remove(struct i2c_client *cl) } static const struct of_device_id arcxcnn_dt_ids[] = { + { .compatible = "arctic,arc2c0608" }, + /* here to remaim compatible with an older binding, do not use */ { .compatible = "arc,arc2c0608" }, { } }; @@ -404,5 +419,5 @@ static struct i2c_driver arcxcnn_driver = { module_i2c_driver(arcxcnn_driver); MODULE_LICENSE("GPL v2"); -MODULE_AUTHOR("Brian Dodge "); +MODULE_AUTHOR("Brian Dodge "); MODULE_DESCRIPTION("ARCXCNN Backlight driver"); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v10
On Tue, Jun 25, 2019 at 07:20:12AM +, Koenig, Christian wrote: > Am 24.06.19 um 16:41 schrieb Daniel Vetter: > > On Mon, Jun 24, 2019 at 03:58:00PM +0200, Christian König wrote: > >> Am 24.06.19 um 13:23 schrieb Koenig, Christian: > >>> Am 21.06.19 um 18:27 schrieb Daniel Vetter: > >>> > So I pondered a few ideas while working out: > > 1) We drop this filtering. Importer needs to keep track of all its > mappings and filter out invalidates that aren't for that specific > importer > (either because already invalidated, or not yet mapped, or whatever). > Feels fragile. > > [SNIP] > >>> [SNIP] > >>> > >>> I will take a moment and look into #1 as well, but I still don't see the > >>> need to change anything. > >> That turned out much cleaner than I thought it would be. Essentially it is > >> only a single extra line of code in amdgpu. > >> > >> Going to send that out as a patch set in a minute. > > Yeah I mean kinda expected that because: > > - everything's protected with ww_mutex anyway > > - importer needs to keep track of mappings anways > > So really all it needs to do is not be stupid and add the mapping it just > > created to its tracking while still holding the ww_mutex. Similar on > > invalidate/unmap. > > > > With that all we need is a huge note in the docs that importers need to > > keep track of their mappings and dtrt (with all the examples here spelled > > out in the appropriate kerneldoc). And then I'm happy :-) > > Should I also rename the invalidate callback into move_notify? Would > kind of make sense since we are not necessary directly invalidating > mappings. Hm maybe. I'll review the entire pile and probably reply with a docs patch anyway. It would help in making it clear you get the callback even when there's no mapping to invalidate for you ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: pwm_bl: Set pin to sleep state when powered down
On Wed, May 22, 2019 at 06:34:28PM +0200, Paul Cercueil wrote: > When the driver probes, the PWM pin is automatically configured to its > default state, which should be the "pwm" function. However, at this > point we don't know the actual level of the pin, which may be active or > inactive. As a result, if the driver probes without enabling the > backlight, the PWM pin might be active, and the backlight would be > lit way before being officially enabled. I'm not sure I understand the problem completely here. Let me try to summarize the problem you solve here: The backlight device's default pinctrl contains the PWM function of the PWM pin. As the PWM is (or at least might be) in an undefined state the default pinctrl should only be switched to when it's clear if the backlight should be on or off. So you use the "init"-pinctrl to keep the PWM pin in some (undriven?) state and by switching to "sleep" you prevent "default" getting active. Did I get this right? If not, please correct me. What is the PWM pin configured to in "init" in your case? Is the pinctrl just empty? Or is it a gpio-mode (together with a gpio-hog)? My thoughts to this is are: a) This is a general problem that applies (I think) to most if not all PWM consumers. If the PWM drives a motor, or makes your mobile vibrate, or drives an LED, or a clk, the PWM shouldn't start to do something before its consumer is ready. b) Thierry made it quite clear[1] that the PWM pin should be configured in a pinctrl of the pwm device, not the backlight (or more general: the consumer) device. While I don't entirely agree with b) I think that even a) alone justifies to think a bit more about the problem and preferably come up with a solution that helps other consumers, too. Ideally if the bootloader sets up the PWM to do something sensible, probing the lowlevel PWM driver and the consumer driver should not interfere with the bootloader's intention until the situation reaches a controlled state. (I.e. if the backlight was left on by the bootloader to show a nice logo, it should not flicker until a userspace program takes over the display device.) A PWM is special in contrast to other devices as its intended behaviour is only fixed once a consumer is present. Without a consumer it is unknown if the PWM is inverted or not. And so the common approach that pinctrl is setup by the device core only doesn't work without drawbacks for PWMs. So if a PWM driver is probing and the PWM hardware already runs at say constant one, some instance must define if the pin is supposed to be configured in its "default" or "sleep" pinctrl. IMHO this isn't possible in general without knowing the polarity of the PWM. (And even if it were known that the polarity is inversed, it might be hard to say if your PWM's hardware doesn't implement a disabled state and has to simulate that using a 0% duty cycle.) Another thing that complicates the matter is that at least pwm-imx27 has the annoying property that disabling it (in hardware) drives the pin low irrespective of the configured polarity. So if you want this type of device to behave properly on disable, it must first drive a 0% duty cycle, then switch the pinctrl state and only then disable the hardware. This rules out that the lowlevel driver is unaware of the pinctrl stuff which would be nice (or an inverted PWM won't be disabled in hardware or you need an ugly sequence of callbacks to disable glitch-free). Also if there is no sleep state, you better don't disable an inversed pwm-imx27 at all (in hardware)? (Alternatively we could drop the (undocumented) guarantee that a disabled PWM results in the pin staying in its idle level.) What are the ways out? I think that if we go and apply your patch, we should at least write some documentation with the details to provide some "standard" way to solve similar problems. Also it might be a good idea to let a PWM know if it is inverted or not such that even without the presence of a consumer it can determine if the hardware is active or not at probe time (in most cases at least). Best regards Uwe [1] https://www.spinics.net/lists/linux-pwm/msg08246.html -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: [PATCH v2 04/15] dt-bindings: display: Convert armadeus,st0700-adapt panel to DT schema
On Mon, Jun 24, 2019 at 03:56:38PM -0600, Rob Herring wrote: > Convert the armadeus,st0700-adapt panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] wake_up_var() memory ordering
Peter Zijlstra wrote: > I tried using wake_up_var() today and accidentally noticed that it > didn't imply an smp_mb() and specifically requires it through > wake_up_bit() / waitqueue_active(). Thinking about it again, I'm not sure why you need to add the barrier when wake_up() (which this is a wrapper around) is required to impose a barrier at the front if there's anything to wake up (ie. the wait queue isn't empty). If this is insufficient, does it make sense just to have wake_up*() functions do an unconditional release or full barrier right at the front, rather than it being conditional on something being woken up? > @@ -619,9 +614,7 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe) > err: > if (!adap->suspend_resume_active) { > adap->active_fe = -1; I'm wondering if there's a missing barrier here. Should the clear_bit() on the next line be clear_bit_unlock() or clear_bit_release()? > - clear_bit(ADAP_SLEEP, &adap->state_bits); > - smp_mb__after_atomic(); > - wake_up_bit(&adap->state_bits, ADAP_SLEEP); > + clear_and_wake_up_bit(ADAP_SLEEP, &adap->state_bits); > } > > dev_dbg(&d->udev->dev, "%s: ret=%d\n", __func__, ret); > diff --git a/fs/afs/fs_probe.c b/fs/afs/fs_probe.c > index cfe62b154f68..377ee07d5f76 100644 > --- a/fs/afs/fs_probe.c > +++ b/fs/afs/fs_probe.c > @@ -18,6 +18,7 @@ static bool afs_fs_probe_done(struct afs_server *server) > > wake_up_var(&server->probe_outstanding); > clear_bit_unlock(AFS_SERVER_FL_PROBING, &server->flags); > + smp_mb__after_atomic(); > wake_up_bit(&server->flags, AFS_SERVER_FL_PROBING); > return true; > } Looking at this and the dvb one, does it make sense to stick the release semantics of clear_bit_unlock() into clear_and_wake_up_bit()? Also, should clear_bit_unlock() be renamed to clear_bit_release() (and similarly test_and_set_bit_lock() -> test_and_set_bit_acquire()) if we seem to be trying to standardise on that terminology. David ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 05/15] dt-bindings: display: Convert bananapi,s070wv20-ct16 panel to DT schema
On Mon, Jun 24, 2019 at 03:56:39PM -0600, Rob Herring wrote: > Convert the bananapi,s070wv20-ct16 panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/15] dt-bindings: display: Convert dlc,dlc0700yzg-1 panel to DT schema
Hi, On Mon, Jun 24, 2019 at 03:56:40PM -0600, Rob Herring wrote: > Convert the dlc,dlc0700yzg-1 panel binding to DT schema. > > Cc: Philipp Zabel > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring > --- > .../display/panel/dlc,dlc0700yzg-1.txt| 13 - > .../display/panel/dlc,dlc0700yzg-1.yaml | 28 +++ > 2 files changed, 28 insertions(+), 13 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.txt > create mode 100644 > Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.yaml > > diff --git > a/Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.txt > b/Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.txt > deleted file mode 100644 > index bf06bb025b08.. > --- a/Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.txt > +++ /dev/null > @@ -1,13 +0,0 @@ > -DLC Display Co. DLC0700YZG-1 7.0" WSVGA TFT LCD panel > - > -Required properties: > -- compatible: should be "dlc,dlc0700yzg-1" > -- power-supply: See simple-panel.txt > - > -Optional properties: > -- reset-gpios: See panel-common.txt > -- enable-gpios: See simple-panel.txt > -- backlight: See simple-panel.txt > - > -This binding is compatible with the simple-panel binding, which is specified > -in simple-panel.txt in this directory. > diff --git > a/Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.yaml > b/Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.yaml > new file mode 100644 > index ..1b0b63d46f3e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/dlc,dlc0700yzg-1.yaml > @@ -0,0 +1,28 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/dlc,dlc0700yzg-1.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: DLC Display Co. DLC0700YZG-1 7.0" WSVGA TFT LCD panel > + > +maintainers: > + - Philipp Zabel > + - Thierry Reding > + > +allOf: > + - $ref: panel-common.yaml# > + > +properties: > + compatible: > +const: dlc,dlc0700yzg-1 > + > + reset-gpios: true > + enable-gpios: true > + backlight: true Do we need to list them? Since we don't have additionalItems, it doesn't really change anything since it will be validated by panel-common. Either way, it should be consistent between your patches, and the previous patches in this series didn't list all the properties in the binding. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/15] dt-bindings: display: Convert pda,91-00156-a0 panel to DT schema
On Mon, Jun 24, 2019 at 03:56:41PM -0600, Rob Herring wrote: > Convert the pda,91-00156-a0 panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 08/15] dt-bindings: display: Convert raspberrypi,7inch-touchscreen panel to DT schema
On Mon, Jun 24, 2019 at 03:56:42PM -0600, Rob Herring wrote: > Convert the raspberrypi,7inch-touchscreen panel binding to DT schema. > > Cc: Eric Anholt > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 13/15] dt-bindings: display: Convert mitsubishi,aa104xd12 panel to DT schema
On Mon, Jun 24, 2019 at 03:56:47PM -0600, Rob Herring wrote: > Convert the mitsubishi,aa104xd12 LVDS panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/15] dt-bindings: display: Convert tfc,s9700rtwv43tr-01b panel to DT schema
On Mon, Jun 24, 2019 at 03:56:43PM -0600, Rob Herring wrote: > Convert the tfc,s9700rtwv43tr-01b panel binding to DT schema. > > Cc: Heiko Stuebner > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring > --- > .../display/panel/tfc,s9700rtwv43tr-01b.txt | 15 -- > .../display/panel/tfc,s9700rtwv43tr-01b.yaml | 30 +++ > 2 files changed, 30 insertions(+), 15 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt > create mode 100644 > Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.yaml > > diff --git > a/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt > b/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt > deleted file mode 100644 > index dfb572f085eb.. > --- > a/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.txt > +++ /dev/null > @@ -1,15 +0,0 @@ > -TFC S9700RTWV43TR-01B 7" Three Five Corp 800x480 LCD panel with > -resistive touch > - > -The panel is found on TI AM335x-evm. > - > -Required properties: > -- compatible: should be "tfc,s9700rtwv43tr-01b" > -- power-supply: See panel-common.txt > - > -Optional properties: > -- enable-gpios: GPIO pin to enable or disable the panel, if there is one > -- backlight: phandle of the backlight device attached to the panel > - > -This binding is compatible with the simple-panel binding, which is specified > -in simple-panel.txt in this directory. > diff --git > a/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.yaml > b/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.yaml > new file mode 100644 > index ..614f4a8d8403 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/panel/tfc,s9700rtwv43tr-01b.yaml > @@ -0,0 +1,30 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/tfc,s9700rtwv43tr-01b.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TFC S9700RTWV43TR-01B 7" Three Five Corp 800x480 LCD panel with > resistive touch > + > +maintainers: > + - Jyri Sarha > + - Thierry Reding > + > +description: |+ > + The panel is found on TI AM335x-evm. > + > +allOf: > + - $ref: panel-common.yaml# > + > +properties: > + compatible: > +const: tfc,s9700rtwv43tr-01b > + > + enable-gpios: true > + backlight: true There's the same remark than on patch 6. Once figured out, Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/15] dt-bindings: display: Convert mitsubishi,aa121td01 panel to DT schema
On Mon, Jun 24, 2019 at 03:56:48PM -0600, Rob Herring wrote: > Convert the mitsubishi,aa121td01 LVDS panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 15/15] dt-bindings: display: Convert sgd, gktw70sdae4se panel to DT schema
On Mon, Jun 24, 2019 at 03:56:49PM -0600, Rob Herring wrote: > Convert the sgd,gktw70sdae4se LVDS panel binding to DT schema. > > Cc: Neil Armstrong > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/15] dt-bindings: display: Convert tpo,tpg110 panel to DT schema
On Mon, Jun 24, 2019 at 03:56:44PM -0600, Rob Herring wrote: > Convert the tpo,tpg110 panel binding to DT schema. > > Cc: Linus Walleij > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH libdrm 0/9] amdgpu:
On 2019-06-24 7:31 p.m., Christian König wrote: > Patches #1 - #3 look good to me, but I'm not sure if the rest is such a > good idea. > > Basically you not only want to use the same FD for CS, but also for > basically all buffer functions and as far as I can see we break that here. How so? The core FD is used for everything except flink and amdgpu_bo_handle_type_kms_user. > I would rather add a new function to export the KMS handle for a certain > BO/FD pair. Exporting the handle based on a type was also like trying to > squeeze a round pig through a square hole in the first place. > > KMS, flink and DMA-buf fd are fundamentally different and shouldn't be > handled by the same function in the first place. I don't really see the problem. radeonsi & RADV are using amdgpu_bo_handle_type_kms to get handles for command stream submission, so those handles have to be valid for the core FD. Now there's amdgpu_bo_handle_type_kms_user to get a handle valid for the FD passed to amdgpu_device_initialize. Can you describe a scenario where a handle valid for yet another file description is needed? -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm/rockchip: Add optional support for CRTC gamma LUT
Hi Ezequiel, On Fri, Jun 21, 2019 at 06:13:45PM -0300, Ezequiel Garcia wrote: > Add an optional CRTC gamma LUT support, and enable it on RK3288. > This is currently enabled via a separate address resource, > which needs to be specified in the devicetree. > > The address resource is required because on some SoCs, such as > RK3288, the LUT address is after the MMU address, and the latter > is supported by a different driver. This prevents the DRM driver > from requesting an entire register space. > > The current implementation works for RGB 10-bit tables, as that > is what seems to work on RK3288. > > Signed-off-by: Ezequiel Garcia > --- > Changes from v1: > * drop explicit linear LUT after finding a proper > way to disable gamma correction. > * avoid setting gamma is the CRTC is not active. > * s/int/unsigned int as suggested by Jacopo. > * only enable color management and set gamma size > if gamma LUT is supported, suggested by Doug. > * drop the reg-names usage, and instead just use indexed reg > specifiers, suggested by Doug. > > Changes from RFC: > * Request (an optional) address resource for the LUT. > * Drop support for RK3399, which doesn't seem to work > out of the box and needs more research. > * Support pass-thru setting when GAMMA_LUT is NULL. > * Add a check for the gamma size, as suggested by Ilia. > * Move gamma setting to atomic_commit_tail, as pointed > out by Jacopo/Laurent, is the correct way. > --- > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 3 + > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 114 > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 7 ++ > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 2 + > 4 files changed, 126 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > index 1c69066b6894..bf9ad6240971 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > @@ -16,6 +16,7 @@ > #include "rockchip_drm_fb.h" > #include "rockchip_drm_gem.h" > #include "rockchip_drm_psr.h" > +#include "rockchip_drm_vop.h" > > static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb, >struct drm_file *file, > @@ -128,6 +129,8 @@ rockchip_atomic_helper_commit_tail_rpm(struct > drm_atomic_state *old_state) > > drm_atomic_helper_commit_modeset_disables(dev, old_state); > > + rockchip_drm_vop_gamma_set(old_state); > + > drm_atomic_helper_commit_modeset_enables(dev, old_state); > > drm_atomic_helper_commit_planes(dev, old_state, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 12ed5265a90b..cfa70773a9bc 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -137,6 +137,7 @@ struct vop { > > uint32_t *regsbak; > void __iomem *regs; > + void __iomem *lut_regs; > > /* physical map length of vop register */ > uint32_t len; > @@ -1153,6 +1154,102 @@ static void vop_wait_for_irq_handler(struct vop *vop) > synchronize_irq(vop->irq); > } > > +static bool vop_dsp_lut_is_enable(struct vop *vop) > +{ > + return vop_read_reg(vop, 0, &vop->data->common->dsp_lut_en); > +} > + > +static void vop_crtc_write_gamma_lut(struct vop *vop, struct drm_crtc *crtc) > +{ > + struct drm_color_lut *lut = crtc->state->gamma_lut->data; > + unsigned int i; > + > + for (i = 0; i < crtc->gamma_size; i++) { > + u32 word; > + > + word = (drm_color_lut_extract(lut[i].red, 10) << 20) | > +(drm_color_lut_extract(lut[i].green, 10) << 10) | > + drm_color_lut_extract(lut[i].blue, 10); > + writel(word, vop->lut_regs + i * 4); > + } > +} > + > +static void vop_crtc_gamma_set(struct vop *vop, struct drm_crtc *crtc, > +struct drm_crtc_state *old_state) > +{ > + unsigned int idle; > + int ret; > + > + /* > + * In order to write the LUT to the internal RAM memory, > + * we need to first make sure the dsp_lut_en bit is cleared. > + */ > + spin_lock(&vop->reg_lock); > + VOP_REG_SET(vop, common, dsp_lut_en, 0); > + vop_cfg_done(vop); > + spin_unlock(&vop->reg_lock); > + > + /* > + * If the CRTC is not active, dsp_lut_en will not get cleared. Did you mean "dsp_lut_en will not get enabled" ? Beacuse I see dsp_lut_en being set to 0, and not activated if !crtc->state->active. Am I confused? Apart from that: Reviewed-by: Jacopo Mondi Thanks j > + * Apparently we still need to do the above step to for > + * gamma correction to be disabled. > + */ > + if (!crtc->state->active) > + return; > + > + ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop, > +idle, !idle, 5, 30 * 1000); > + if (ret) { > + DRM_DEV_ERROR(vop->dev,
Re: [RFC][PATCH] wake_up_var() memory ordering
(sorry for cross-posting to moderated lists btw, I've since acquired a patch to get_maintainers.pl that wil exclude them in the future) On Tue, Jun 25, 2019 at 08:51:01AM +0100, David Howells wrote: > Peter Zijlstra wrote: > > > I tried using wake_up_var() today and accidentally noticed that it > > didn't imply an smp_mb() and specifically requires it through > > wake_up_bit() / waitqueue_active(). > > Thinking about it again, I'm not sure why you need to add the barrier when > wake_up() (which this is a wrapper around) is required to impose a barrier at > the front if there's anything to wake up (ie. the wait queue isn't empty). > > If this is insufficient, does it make sense just to have wake_up*() functions > do an unconditional release or full barrier right at the front, rather than it > being conditional on something being woken up? The curprit is __wake_up_bit()'s usage of waitqueue_active(); it is this latter (see its comment) that requires the smp_mb(). wake_up_bit() and wake_up_var() are wrappers around __wake_up_bit(). Without this barrier it is possible for the waitqueue_active() load to be hoisted over the cond=true store and the remote end can miss the store and we can miss its enqueue and we'll all miss a wakeup and get stuck. Adding an smp_mb() (or use wq_has_sleeper()) in __wake_up_bit() would be nice, but I fear some people will complain about overhead, esp. since about half the sites don't need the barrier due to being behind test_and_clear_bit() and the other half using smp_mb__after_atomic() after some clear_bit*() variant. There's a few sites that seem to open-code wait_var_event()/wake_up_var() and those actually need the full smp_mb(), but then maybe they should be converted to var instread of bit anyway. > > @@ -619,9 +614,7 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe) > > err: > > if (!adap->suspend_resume_active) { > > adap->active_fe = -1; > > I'm wondering if there's a missing barrier here. Should the clear_bit() on > the next line be clear_bit_unlock() or clear_bit_release()? That looks reasonable, but I'd like to hear from the DVB folks on that. > > - clear_bit(ADAP_SLEEP, &adap->state_bits); > > - smp_mb__after_atomic(); > > - wake_up_bit(&adap->state_bits, ADAP_SLEEP); > > + clear_and_wake_up_bit(ADAP_SLEEP, &adap->state_bits); > > } > > > > dev_dbg(&d->udev->dev, "%s: ret=%d\n", __func__, ret); > > diff --git a/fs/afs/fs_probe.c b/fs/afs/fs_probe.c > > index cfe62b154f68..377ee07d5f76 100644 > > --- a/fs/afs/fs_probe.c > > +++ b/fs/afs/fs_probe.c > > @@ -18,6 +18,7 @@ static bool afs_fs_probe_done(struct afs_server *server) > > > > wake_up_var(&server->probe_outstanding); > > clear_bit_unlock(AFS_SERVER_FL_PROBING, &server->flags); > > + smp_mb__after_atomic(); > > wake_up_bit(&server->flags, AFS_SERVER_FL_PROBING); > > return true; > > } > > Looking at this and the dvb one, does it make sense to stick the release > semantics of clear_bit_unlock() into clear_and_wake_up_bit()? I was thinking of adding another helper, maybe unlock_and_wake_up_bit() that included that extra barrier, but maybe making it unconditional isn't the worst idea. > Also, should clear_bit_unlock() be renamed to clear_bit_release() (and > similarly test_and_set_bit_lock() -> test_and_set_bit_acquire()) if we seem to > be trying to standardise on that terminology. That definitely makes sense to me, there's only 157 clear_bit_unlock() and 76 test_and_set_bit_lock() users (note the asymetry of that). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/mcde: Fix uninitialized variable
On Tue, Jun 25, 2019 at 9:14 AM Dan Carpenter wrote: > On Tue, Jun 25, 2019 at 12:09:54AM +0200, Linus Walleij wrote: > > On Tue, Jun 18, 2019 at 2:31 PM Dan Carpenter > > wrote: > > > > > Thanks! > > > > Recording this as Acked-by: when applying. > > > > That's is an honour for me. I figured that your Signed-off-by dwarfed > my Ack. :P Not at all. In DRM we have a rule to not push any changes without a proper ACK. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/15] dt-bindings: display: Convert tpo,tpg110 panel to DT schema
On Tue, Jun 25, 2019 at 12:47 AM Rob Herring wrote: > On Mon, Jun 24, 2019 at 4:13 PM Linus Walleij > wrote: > > On Mon, Jun 24, 2019 at 11:59 PM Rob Herring wrote: > > > > > Convert the tpo,tpg110 panel binding to DT schema. > > > > > > Cc: Linus Walleij > > > Cc: Thierry Reding > > > Cc: Sam Ravnborg > > > Cc: Maxime Ripard > > > Cc: Laurent Pinchart > > > Cc: dri-devel@lists.freedesktop.org > > > Signed-off-by: Rob Herring > > > > Awesome, fixed up the MAINATINERS entry and applied and > > pushed for DRM next with my Reviewed-by. > > You shouldn't have because this is dependent on patch 2 and > panel-common.yaml. So now 'make dt_binding_check' is broken. Ooops easily happens when I am only adressee on this patch and there is no mention of any dependencies. Can I simply just merge the panel-common patch as well and we are all happy? I can also pick up more panel binding patches, IMO the yaml conversions are especially uncontroversial and should have low threshold for merging. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110984] Vulkan shows stuttering issues on Vega 10 w/ gnome-shell on Wayland
https://bugs.freedesktop.org/show_bug.cgi?id=110984 Michel Dänzer changed: What|Removed |Added Resolution|--- |NOTOURBUG Status|NEW |RESOLVED --- Comment #1 from Michel Dänzer --- This is an Xwayland issue, due to it only using a single buffer for sending contents to the Wayland server. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: backlight: fix vendor prefix for ArcticSand arcxcnn driver bindings
On Tue, Jun 25, 2019 at 12:05:28AM -0400, Brian Dodge wrote: > The vendor-prefixes.txt file properly refers to ArcticSand > as arctic but the driver bindings improperly abbreviated the > prefix to arc. This was a mistake in the original patch > > Signed-off-by: Brian Dodge > --- > .../bindings/leds/backlight/arcxcnn_bl.txt | 24 > +- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > index 230abde..9cf4c44 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > @@ -1,8 +1,12 @@ > -Binding for ArcticSand arc2c0608 LED driver > +Binding for ArcticSand arc family LED drivers > > Required properties: > -- compatible:should be "arc,arc2c0608" > -- reg: slave address > +- compatible: one of > + "arctic,arc1c0608" > + "arctic,arc2c0608" > + "arctic,arc3c0845" This is more a question for the DT folks than for Brian but... AFAICT this patch is fixing the binding for the ArcticSand devices to use the correct value from vendor-prefixes.yaml and has been previously discussed here: https://lkml.org/lkml/2018/9/25/726 Currently this patch series just updates the DT bindings but the implementation also honours the old values (since there is a Chromebook in the wild that uses the current bindings). Hence I'm not clear whether the bindings should document the deprecated options too (e.g. make it easier to find the bindings doc with git grep and friends). Daniel. > + > +- reg: slave address > > Optional properties: > - default-brightness:brightness value on boot, value from: 0-4095 > @@ -11,19 +15,19 @@ Optional properties: > - led-sources: List of enabled channels from 0 to 5. > See Documentation/devicetree/bindings/leds/common.txt > > -- arc,led-config-0: setting for register ILED_CONFIG_0 > -- arc,led-config-1: setting for register ILED_CONFIG_1 > -- arc,dim-freq: PWM mode frequence setting (bits [3:0] used) > -- arc,comp-config: setting for register CONFIG_COMP > -- arc,filter-config: setting for register FILTER_CONFIG > -- arc,trim-config: setting for register IMAXTUNE > +- arctic,led-config-0: setting for register ILED_CONFIG_0 > +- arctic,led-config-1: setting for register ILED_CONFIG_1 > +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used) > +- arctic,comp-config:setting for register CONFIG_COMP > +- arctic,filter-config: setting for register FILTER_CONFIG > +- arctic,trim-config:setting for register IMAXTUNE > > Note: Optional properties not specified will default to values in IC EPROM > > Example: > > arc2c0608@30 { > - compatible = "arc,arc2c0608"; > + compatible = "arctic,arc2c0608"; > reg = <0x30>; > default-brightness = <500>; > label = "lcd-backlight"; > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] wake_up_var() memory ordering
Peter, thanks for the fixes. On Mon, 24 Jun 2019 at 18:53, Peter Zijlstra wrote: > Hi all, > > I tried using wake_up_var() today and accidentally noticed that it > didn't imply an smp_mb() and specifically requires it through > wake_up_bit() / waitqueue_active(). > > Now, wake_up_bit() doesn't imply the barrier because it is assumed to be > used with the atomic bitops API which either implies (test_and_clear) or > only needs smp_mb__after_atomic(), which is 'much' cheaper than an > unconditional smp_mb(). > > Still, while auditing all that, I found a whole bunch of things that > could be improved. There were missing barriers, superfluous barriers and > a whole bunch of sites that could use clear_and_wake_up_bit(). > > So this fixes all wake_up_bit() usage without actually changing > semantics of it (which are unfortunate but understandable). thanks for the fixes. > This does > however change the semantics of wake_up_var(); even though wake_up_var() > is most often used with atomics and then the additional smp_mb() is most > often superfluous :/ > > There isn't really a good option here, comments (other than I need to > split this up)? > > > --- > drivers/bluetooth/btmtksdio.c | 5 + > drivers/bluetooth/btmtkuart.c | 5 + > drivers/bluetooth/hci_mrvl.c| 8 ++-- > drivers/gpu/drm/i915/i915_reset.c | 6 ++ > drivers/md/dm-bufio.c | 10 ++ > drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 15 --- > fs/afs/fs_probe.c | 1 + > fs/afs/server.c | 1 + > fs/afs/vl_probe.c | 1 + > fs/afs/volume.c | 1 + > fs/aio.c| 4 +--- > fs/block_dev.c | 1 + > fs/btrfs/extent_io.c| 4 +--- > fs/cachefiles/namei.c | 1 + > fs/cifs/connect.c | 3 +-- > fs/cifs/misc.c | 15 +-- > fs/fscache/cookie.c | 2 ++ > fs/fscache/object.c | 2 ++ > fs/fscache/page.c | 3 +++ > fs/gfs2/glock.c | 8 ++-- > fs/gfs2/glops.c | 1 + > fs/gfs2/lock_dlm.c | 8 ++-- > fs/gfs2/recovery.c | 4 +--- > fs/gfs2/super.c | 1 + > fs/gfs2/sys.c | 4 +--- > fs/nfs/nfs4state.c | 4 +--- > fs/nfs/pnfs_nfs.c | 4 +--- > fs/nfsd/nfs4recover.c | 4 +--- > fs/orangefs/file.c | 2 +- > kernel/sched/wait_bit.c | 1 + > net/bluetooth/hci_event.c | 5 + > net/rds/ib_recv.c | 1 + > security/keys/gc.c | 5 ++--- > 33 files changed, 50 insertions(+), 90 deletions(-) > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > index 813338288453..27523cfeac9a 100644 > --- a/drivers/bluetooth/btmtksdio.c > +++ b/drivers/bluetooth/btmtksdio.c > @@ -356,11 +356,8 @@ static int btmtksdio_recv_event(struct hci_dev *hdev, > struct sk_buff *skb) > > if (hdr->evt == HCI_EV_VENDOR) { > if (test_and_clear_bit(BTMTKSDIO_TX_WAIT_VND_EVT, > - &bdev->tx_state)) { > - /* Barrier to sync with other CPUs */ > - smp_mb__after_atomic(); > + &bdev->tx_state)) > wake_up_bit(&bdev->tx_state, > BTMTKSDIO_TX_WAIT_VND_EVT); > - } > } > > return 0; > diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c > index f5dbeec8e274..7fe324df3799 100644 > --- a/drivers/bluetooth/btmtkuart.c > +++ b/drivers/bluetooth/btmtkuart.c > @@ -340,11 +340,8 @@ static int btmtkuart_recv_event(struct hci_dev *hdev, > struct sk_buff *skb) > > if (hdr->evt == HCI_EV_VENDOR) { > if (test_and_clear_bit(BTMTKUART_TX_WAIT_VND_EVT, > - &bdev->tx_state)) { > - /* Barrier to sync with other CPUs */ > - smp_mb__after_atomic(); > + &bdev->tx_state)) > wake_up_bit(&bdev->tx_state, > BTMTKUART_TX_WAIT_VND_EVT); > - } > } > > return 0; > diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c > index 50212ac629e3..f03294d39d08 100644 > --- a/drivers/bluetooth/hci_mrvl.c > +++ b/drivers/bluetooth/hci_mrvl.c > @@ -157,9 +157,7 @@ static int mrvl_recv_fw_req(struct hci_dev *hdev, struct > sk_buff *skb) > > mrvl->tx_len = le16_
Re: [PATCH] backlight: pwm_bl: Set pin to sleep state when powered down
On Mon, Jun 24, 2019 at 12:28:44PM +0100, Daniel Thompson wrote: > On Fri, Jun 21, 2019 at 03:56:08PM +0200, Thierry Reding wrote: > > On Fri, Jun 21, 2019 at 01:41:45PM +0100, Daniel Thompson wrote: > > > On 22/05/2019 17:34, Paul Cercueil wrote: > > > > When the driver probes, the PWM pin is automatically configured to its > > > > default state, which should be the "pwm" function. > > > > > > At which point in the probe... and by who? > > > > The driver core will select the "default" state of a device right before > > calling the driver's probe, see: > > > > drivers/base/pinctrl.c: pinctrl_bind_pins() > > > > which is called from: > > > > drivers/base/dd.c: really_probe() > > > > Thanks. I assumed it would be something like that... although given > pwm-backlight is essentially a wrapper driver round a PWM I wondered why > the pinctrl was on the backlight node (rather than the PWM node). I agree with this. We're defining the pin control state for the PWM pin, so in my opinion it should be the PWM driver that controls it. One reason why I think this is important is if we ever end up with a device that requires pins from two different controllers to be configured at runtime, then how would we model that? Since pin control states cannot be aggregated, so you'd have to have multiple "default" states, each for the pins that they control. On the other hand if we associate the pin control states with each of the resources that need those states, then when those resources are controlled, they will automatically know how to deal with the states. The top-level device (i.e. backlight) doesn't need to concern itself with those details. > Looking at the DTs in the upstream kernel it looks like ~20% of the > backlight drivers have pinctrl on the backlight node. Others presumable > have none or have it on the PWM node (and it looks like support for > sleeping the pins is *very* rare amoung the PWM drivers). I suspect that that's mostly a sign of our device trees and kernel subsystems still maturing. For example, I think it's fairly rare for a device to seamlessly take over the display configuration from the bootloader. Most of the time you'll just see things go black (that's actually one of the better cases) when the kernel takes over and then the backlight will come up again at some point. Taking over the bootloader's display configuration is pretty hard and there are numerous pieces to the puzzle (need to make sure clocks and power supplies are not automatically disabled after the initcalls, display drivers need to know how to read out hardware, claim whatever memory region the bootloader was using for a bootsplash, backlight is supposed to remain enabled if the bootloader turned it on, ...). I don't think the fact that PWM drivers don't support this implies that hardware doesn't support it. I think we've just never needed it before because we get away with it. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: Attach to DSI host at probe time
Hi, Any feedback on this patch? Thanks, Matt On 24/04/2019 14:22, Matthew Redfearn wrote: > In contrast to all of the DSI panel drivers in drivers/gpu/drm/panel > which attach to the DSI host via mipi_dsi_attach() at probe time, the > ADV7533 bridge device does not. Instead it defers this to the point that > the upstream device connects to its bridge via drm_bridge_attach(). > The generic Synopsys MIPI DSI host driver does not register it's own > drm_bridge until the MIPI DSI has attached. But it does not call > drm_bridge_attach() on the downstream device until the upstream device > has attached. This leads to a chicken and the egg failure and the DRM > pipeline does not complete. > Since all other mipi_dsi_device drivers call mipi_dsi_attach() in > probe(), make the adv7533 mipi_dsi_device do the same. This ensures that > the Synopsys MIPI DSI host registers it's bridge such that it is > available for the upstream device to connect to. > > Signed-off-by: Matt Redfearn > > --- > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index e7ddd3e3db9..ea36ac3a3de 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -874,9 +874,6 @@ static int adv7511_bridge_attach(struct drm_bridge > *bridge) >&adv7511_connector_helper_funcs); > drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > - if (adv->type == ADV7533) > - ret = adv7533_attach_dsi(adv); > - > if (adv->i2c_main->irq) > regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0), >ADV7511_INT0_HPD); > @@ -1222,7 +1219,11 @@ static int adv7511_probe(struct i2c_client *i2c, const > struct i2c_device_id *id) > drm_bridge_add(&adv7511->bridge); > > adv7511_audio_init(dev, adv7511); > - return 0; > + > + if (adv7511->type == ADV7533) > + return adv7533_attach_dsi(adv7511); > + else > + return 0; > > err_unregister_cec: > i2c_unregister_device(adv7511->i2c_cec); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 0/9] amdgpu:
Am 25.06.19 um 10:02 schrieb Michel Dänzer: > On 2019-06-24 7:31 p.m., Christian König wrote: >> Patches #1 - #3 look good to me, but I'm not sure if the rest is such a >> good idea. >> >> Basically you not only want to use the same FD for CS, but also for >> basically all buffer functions and as far as I can see we break that here. > How so? The core FD is used for everything except flink and > amdgpu_bo_handle_type_kms_user. IIRC in the Mesa winsys we compare the amdgpu_device and amdgpu_bo pointers to figure out if an opened device or imported BO is the same as one we already know. With this patch that won't work any more and for example OpenGL and VA-API could potentially use separate amdgpu_bo pointers for the same underlying buffer. That in turn would break synchronization. Additional to that we potentially could keep a bunch of file descriptors around which we don't necessary want. In other words we previously closed the extra file descriptors, but now we keep them open as well. But that should probably be only a minor problem. >> I would rather add a new function to export the KMS handle for a certain >> BO/FD pair. Exporting the handle based on a type was also like trying to >> squeeze a round pig through a square hole in the first place. >> >> KMS, flink and DMA-buf fd are fundamentally different and shouldn't be >> handled by the same function in the first place. > I don't really see the problem. radeonsi & RADV are using > amdgpu_bo_handle_type_kms to get handles for command stream submission, > so those handles have to be valid for the core FD. Now there's > amdgpu_bo_handle_type_kms_user to get a handle valid for the FD passed > to amdgpu_device_initialize. Can you describe a scenario where a handle > valid for yet another file description is needed? Not yet another file descriptor, but the underlying problem here seems to be that we don't tell libdrm in which domain/file descriptor the KMS should actually be valid in. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] Associate ddc adapters with connectors
It is difficult for a user to know which of the i2c adapters is for which drm connector. This series addresses this problem. The idea is to have a symbolic link in connector's sysfs directory, e.g.: ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2 lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \ -> ../../../../soc/1388.i2c/i2c-2 The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run ddcutil: ddcutil -b 2 getvcp 0x10 VCP code 0x10 (Brightness): current value =90, max value = 100 The first patch in the series adds struct i2c_adapter pointer to struct drm_connector. If the field is used by a particular driver, then an appropriate symbolic link is created by the generic code, which is also added by this patch. The second patch is an example of how to convert a driver to this new scheme. Andrzej Pietrasiewicz (2): drm: Include ddc adapter pointer in struct drm_connector drm/exynos: Provide ddc symlink in connector's sysfs drivers/gpu/drm/drm_sysfs.c | 9 + drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +-- include/drm/drm_connector.h | 11 +++ 3 files changed, 25 insertions(+), 6 deletions(-) -- 2.17.1
[PATCH 1/2] drm: Include ddc adapter pointer in struct drm_connector
Add generic code which creates symbolic links in sysfs, pointing to ddc interface used by a particular video output. For example: ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2 lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \ -> ../../../../soc/1388.i2c/i2c-2 This makes it easy for user to associate a display with its ddc adapter and use e.g. ddcutil to control the chosen monitor. This patch adds an i2c_adapter pointer to struct drm_connector. Particular drivers can then use it instead of using their own private instance. If a connector contains a ddc, then create a symbolic link in sysfs. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/drm_sysfs.c | 9 + include/drm/drm_connector.h | 11 +++ 2 files changed, 20 insertions(+) diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index ad10810bc972..627f8ebfc87a 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -294,6 +294,10 @@ int drm_sysfs_connector_add(struct drm_connector *connector) /* Let userspace know we have a new connector */ drm_sysfs_hotplug_event(dev); + if (connector->ddc) + return sysfs_create_link(&connector->kdev->kobj, +&connector->ddc->dev.kobj, +connector->ddc->dev.kobj.name); return 0; } @@ -301,6 +305,11 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) { if (!connector->kdev) return; + + if (connector->ddc) + sysfs_remove_link(&connector->kdev->kobj, + connector->ddc->dev.kobj.name); + DRM_DEBUG("removing \"%s\" from sysfs\n", connector->name); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index ca745d9feaf5..1ad3d1d54ba7 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -23,6 +23,7 @@ #ifndef __DRM_CONNECTOR_H__ #define __DRM_CONNECTOR_H__ +#include #include #include #include @@ -1308,6 +1309,16 @@ struct drm_connector { * [0]: progressive, [1]: interlaced */ int audio_latency[2]; + + /** +* @ddc: associated ddc adapter. +* A connector usually has its associated ddc adapter. If a driver uses +* this field, then an appropriate symbolic link is created in connector +* sysfs directory to make it easy for the user to tell which i2c +* adapter is for a particular display. +*/ + struct i2c_adapter *ddc; + /** * @null_edid_counter: track sinks that give us all zeros for the EDID. * Needed to workaround some HW bugs where we get all 0s -- 2.17.1
[PATCH 2/2] drm/exynos: Provide ddc symlink in connector's sysfs
Switch to using the ddc provided by the generic connector. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 894a99793633..6816e37861b7 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -126,7 +126,6 @@ struct hdmi_context { void __iomem*regs; void __iomem*regs_hdmiphy; struct i2c_client *hdmiphy_port; - struct i2c_adapter *ddc_adpt; struct gpio_desc*hpd_gpio; int irq; struct regmap *pmureg; @@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector) struct edid *edid; int ret; - if (!hdata->ddc_adpt) + if (!connector->ddc) return -ENODEV; - edid = drm_get_edid(connector, hdata->ddc_adpt); + edid = drm_get_edid(connector, connector->ddc); if (!edid) return -ENODEV; @@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata) return -EPROBE_DEFER; } - hdata->ddc_adpt = adpt; + hdata->connector.ddc = adpt; return 0; } @@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev) if (hdata->regs_hdmiphy) iounmap(hdata->regs_hdmiphy); err_ddc: - put_device(&hdata->ddc_adpt->dev); + put_device(&hdata->connector.ddc->dev); return ret; } @@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev) if (hdata->regs_hdmiphy) iounmap(hdata->regs_hdmiphy); - put_device(&hdata->ddc_adpt->dev); + put_device(&hdata->connector.ddc->dev); mutex_destroy(&hdata->mutex); -- 2.17.1
Re: [PATCH] backlight: pwm_bl: Set pin to sleep state when powered down
On Mon, Jun 24, 2019 at 04:31:57PM +0200, Paul Cercueil wrote: > > > Le lun. 24 juin 2019 à 13:28, Daniel Thompson a > écrit : > > On Fri, Jun 21, 2019 at 03:56:08PM +0200, Thierry Reding wrote: > > > On Fri, Jun 21, 2019 at 01:41:45PM +0100, Daniel Thompson wrote: > > > > On 22/05/2019 17:34, Paul Cercueil wrote: > > > > > When the driver probes, the PWM pin is automatically configured > > > to its > > > > > default state, which should be the "pwm" function. > > > > > > > > At which point in the probe... and by who? > > > > > > The driver core will select the "default" state of a device right > > > before > > > calling the driver's probe, see: > > > > > > drivers/base/pinctrl.c: pinctrl_bind_pins() > > > > > > which is called from: > > > > > > drivers/base/dd.c: really_probe() > > > > > > > Thanks. I assumed it would be something like that... although given > > pwm-backlight is essentially a wrapper driver round a PWM I wondered why > > the pinctrl was on the backlight node (rather than the PWM node). > > > > Looking at the DTs in the upstream kernel it looks like ~20% of the > > backlight drivers have pinctrl on the backlight node. Others presumable > > have none or have it on the PWM node (and it looks like support for > > sleeping the pins is *very* rare amoung the PWM drivers). > > If your PWM driver has more than one channel and has the pinctrl node, you > cannot fine-tune the state of individual pins. They all share the same > state. But that's something that could be changed, right? We could for example extend the PWM bindings to allow describing each PWM instance via a sub- node of the controller node. Pin control states could be described on a per-channel basis that way. > > > > > However, at this > > > > > point we don't know the actual level of the pin, which may be > > > active or > > > > > inactive. As a result, if the driver probes without enabling the > > > > > backlight, the PWM pin might be active, and the backlight would > > > be > > > > > lit way before being officially enabled. > > > > > > > > > > To work around this, if the probe function doesn't enable the > > > backlight, > > > > > the pin is set to its sleep state instead of the default one, > > > until the > > > > > backlight is enabled. Whenk the backlight is disabled, the pin > > > is reset > > > > > to its sleep state. > > > > Doesn't this workaround result in a backlight flash between > > > whatever enables > > > > it and the new code turning it off again? > > > > > > Yeah, I think it would. I guess if you're very careful on how you > > > set up > > > the device tree you might be able to work around it. Besides the > > > default > > > and idle standard pinctrl states, there's also the "init" state. The > > > core will select that instead of the default state if available. > > > However > > > there's also pinctrl_init_done() which will try again to switch to > > > the > > > default state after probe has finished and the driver didn't switch > > > away > > > from the init state. > > > > > > So you could presumably set up the device tree such that you have > > > three > > > states defined: "default" would be the one where the PWM pin is > > > active, > > > "idle" would be used when backlight is off (PWM pin inactive) and > > > then > > > another "init" state that would be the same as "idle" to be used > > > during > > > probe. During probe the driver could then switch to the "idle" > > > state so > > > that the pin shouldn't glitch. > > > > > > I'm not sure this would actually work because I think the way that > > > pinctrl handles states both "init" and "idle" would be the same > > > pointer > > > values and therefore pinctrl_init_done() would think the driver > > > didn't > > > change away from the "init" state because it is the same pointer > > > value > > > as the "idle" state that the driver selected. One way to work around > > > that would be to duplicate the "idle" state definition and > > > associate one > > > instance of it with the "idle" state and the other with the "init" > > > state. At that point both states should be different (different > > > pointer > > > values) and we'd get the init state selected automatically before > > > probe, > > > select "idle" during probe and then the core will leave it alone. > > > That's > > > of course ugly because we duplicate the pinctrl state in DT, but > > > perhaps > > > it's the least ugly solution. > > > Adding Linus for visibility. Perhaps he can share some insight. > > > > To be honest I'm happy to summarize in my head as "if it flashes then > > it's not > > a pwm_bl.c's problem" ;-). > > It does not flash. But the backlight lits way too early, so we have a 1-2 > seconds > of "white screen" before the panel driver starts. I think this always goes both ways. If you set the sleep state for the PWM on backlight probe with this patch, you may be able to work around the problem of the backlight lighting up too
Re: [PATCH] backlight: pwm_bl: Set pin to sleep state when powered down
On Tue, Jun 25, 2019 at 09:42:20AM +0200, Uwe Kleine-König wrote: > On Wed, May 22, 2019 at 06:34:28PM +0200, Paul Cercueil wrote: > > When the driver probes, the PWM pin is automatically configured to its > > default state, which should be the "pwm" function. However, at this > > point we don't know the actual level of the pin, which may be active or > > inactive. As a result, if the driver probes without enabling the > > backlight, the PWM pin might be active, and the backlight would be > > lit way before being officially enabled. > > I'm not sure I understand the problem completely here. Let me try to > summarize the problem you solve here: > > The backlight device's default pinctrl contains the PWM function of the > PWM pin. As the PWM is (or at least might be) in an undefined state the > default pinctrl should only be switched to when it's clear if the > backlight should be on or off. > > So you use the "init"-pinctrl to keep the PWM pin in some (undriven?) > state and by switching to "sleep" you prevent "default" getting active. > > Did I get this right? If not, please correct me. > > What is the PWM pin configured to in "init" in your case? Is the pinctrl > just empty? Or is it a gpio-mode (together with a gpio-hog)? > > My thoughts to this is are: > > a) This is a general problem that applies (I think) to most if not all > PWM consumers. If the PWM drives a motor, or makes your mobile > vibrate, or drives an LED, or a clk, the PWM shouldn't start > to do something before its consumer is ready. Yes, it shouldn't start before it is explicitly told to do so by the consumer. One exception is if the PWM was already set up by firmware to run. So I think in general terms we always want the PWM to remain in the current state upon probe. The atomic PWM API was designed with that in mind. The original use- case was to allow seamlessly taking over from a PWM regulator. In order to do so, the driver needs to be able to read back the hardware state and *not* initialize the PWM to some default state. I think that same approach can be extended to backlights. The driver's probe needs to determine what the current state of the backlight is and preferable not touch it. And that basically propagates all the way to the display driver, which ultimately needs to determine whether or not the display configuration (including the backlight) is enabled. > b) Thierry made it quite clear[1] that the PWM pin should be configured > in a pinctrl of the pwm device, not the backlight (or more general: > the consumer) device. > > While I don't entirely agree with b) I think that even a) alone > justifies to think a bit more about the problem and preferably come up > with a solution that helps other consumers, too. Ideally if the > bootloader sets up the PWM to do something sensible, probing the > lowlevel PWM driver and the consumer driver should not interfere with > the bootloader's intention until the situation reaches a controlled > state. (I.e. if the backlight was left on by the bootloader to show a > nice logo, it should not flicker until a userspace program takes over > the display device.) Yes, exactly that. > A PWM is special in contrast to other devices as its intended behaviour > is only fixed once a consumer is present. Without a consumer it is > unknown if the PWM is inverted or not. And so the common approach that > pinctrl is setup by the device core only doesn't work without drawbacks > for PWMs. Actually I don't think PWMs are special in this regard. A GPIO, for example, can also be active-low or active-high, and without a consumer there's not enough context to determine which one it should be. So this is really a more general problem. Whenever you want to take over some bootloader configuration, basically all of your OS drivers have to be taught to handle it properly, which usually means not touching any hardware at probe time, preferably reading back the current hardware state and "apply the delta" once the consumer says so. Thierry > So if a PWM driver is probing and the PWM hardware already runs at say > constant one, some instance must define if the pin is supposed to be > configured in its "default" or "sleep" pinctrl. IMHO this isn't possible > in general without knowing the polarity of the PWM. (And even if it were > known that the polarity is inversed, it might be hard to say if your > PWM's hardware doesn't implement a disabled state and has to simulate > that using a 0% duty cycle.) > > Another thing that complicates the matter is that at least pwm-imx27 has > the annoying property that disabling it (in hardware) drives the pin low > irrespective of the configured polarity. So if you want this type of > device to behave properly on disable, it must first drive a 0% duty > cycle, then switch the pinctrl state and only then disable the hardware. > This rules out that the lowlevel driver is unaware of the pinctrl stuff > which would be nice (or an inverted PWM won't
Re: [PATCH 0/2] Associate ddc adapters with connectors
On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote: > It is difficult for a user to know which of the i2c adapters is for which > drm connector. This series addresses this problem. > > The idea is to have a symbolic link in connector's sysfs directory, e.g.: > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2 > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \ > -> ../../../../soc/1388.i2c/i2c-2 Don't you want the symlink name to be "i2c" or something fixed, rather than the name of the i2c adapter? Otherwise, you seem to be encumbering userspace with searching the directory to try and find the symlink. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH libdrm 0/9] amdgpu:
On 2019-06-25 11:44 a.m., Koenig, Christian wrote: > Am 25.06.19 um 10:02 schrieb Michel Dänzer: >> On 2019-06-24 7:31 p.m., Christian König wrote: >>> Patches #1 - #3 look good to me, but I'm not sure if the rest is such a >>> good idea. >>> >>> Basically you not only want to use the same FD for CS, but also for >>> basically all buffer functions and as far as I can see we break that here. >> How so? The core FD is used for everything except flink and >> amdgpu_bo_handle_type_kms_user. > > IIRC in the Mesa winsys we compare the amdgpu_device and amdgpu_bo > pointers to figure out if an opened device or imported BO is the same as > one we already know. > > With this patch that won't work any more and for example OpenGL and > VA-API could potentially use separate amdgpu_bo pointers for the same > underlying buffer. That in turn would break synchronization. Hmm, I was hoping patch 6 would cover this, but it looks like OpenGL and VA-API pass file descriptors referencing different file descriptions to amdgpu_device_initialize, so you're right. :( >>> I would rather add a new function to export the KMS handle for a certain >>> BO/FD pair. I'll try this approach then. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] Associate ddc adapters with connectors
On Tue, Jun 25, 2019 at 12:14:27PM +0200, Andrzej Pietrasiewicz wrote: > Hi Russell, > > W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze: > > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote: > > > It is difficult for a user to know which of the i2c adapters is for which > > > drm connector. This series addresses this problem. > > > > > > The idea is to have a symbolic link in connector's sysfs directory, e.g.: > > > > > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2 > > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 > > > \ > > > -> ../../../../soc/1388.i2c/i2c-2 > > > > Don't you want the symlink name to be "i2c" or something fixed, rather > > than the name of the i2c adapter? Otherwise, you seem to be encumbering > > userspace with searching the directory to try and find the symlink. > > > > Thank you for your comment. So you imagine something on the lines of: > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \ > -> ../../../../soc/1388.i2c/i2c-2 > > ? Exactly. > This makes sense to me, I will send a v2. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
[PATCH v2 0/2] Associate ddc adapters with connectors
It is difficult for a user to know which of the i2c adapters is for which drm connector. This series addresses this problem. The idea is to have a symbolic link in connector's sysfs directory, e.g.: ls -l /sys/class/drm/card0-HDMI-A-1/ddc lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \ -> ../../../../soc/1388.i2c/i2c-2 The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run ddcutil: ddcutil -b 2 getvcp 0x10 VCP code 0x10 (Brightness): current value =90, max value = 100 The first patch in the series adds struct i2c_adapter pointer to struct drm_connector. If the field is used by a particular driver, then an appropriate symbolic link is created by the generic code, which is also added by this patch. The second patch is an example of how to convert a driver to this new scheme. v1..v2: - used fixed name "ddc" for the symbolic link in order to make it easy for userspace to find the i2c adapter Andrzej Pietrasiewicz (2): drm: Include ddc adapter pointer in struct drm_connector drm/exynos: Provide ddc symlink in connector's sysfs drivers/gpu/drm/drm_sysfs.c | 7 +++ drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +-- include/drm/drm_connector.h | 11 +++ 3 files changed, 23 insertions(+), 6 deletions(-) -- 2.17.1
[PATCH v2 2/2] drm/exynos: Provide ddc symlink in connector's sysfs
Switch to using the ddc provided by the generic connector. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 894a99793633..6816e37861b7 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -126,7 +126,6 @@ struct hdmi_context { void __iomem*regs; void __iomem*regs_hdmiphy; struct i2c_client *hdmiphy_port; - struct i2c_adapter *ddc_adpt; struct gpio_desc*hpd_gpio; int irq; struct regmap *pmureg; @@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector) struct edid *edid; int ret; - if (!hdata->ddc_adpt) + if (!connector->ddc) return -ENODEV; - edid = drm_get_edid(connector, hdata->ddc_adpt); + edid = drm_get_edid(connector, connector->ddc); if (!edid) return -ENODEV; @@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata) return -EPROBE_DEFER; } - hdata->ddc_adpt = adpt; + hdata->connector.ddc = adpt; return 0; } @@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev) if (hdata->regs_hdmiphy) iounmap(hdata->regs_hdmiphy); err_ddc: - put_device(&hdata->ddc_adpt->dev); + put_device(&hdata->connector.ddc->dev); return ret; } @@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev) if (hdata->regs_hdmiphy) iounmap(hdata->regs_hdmiphy); - put_device(&hdata->ddc_adpt->dev); + put_device(&hdata->connector.ddc->dev); mutex_destroy(&hdata->mutex); -- 2.17.1
[PATCH v2 1/2] drm: Include ddc adapter pointer in struct drm_connector
Add generic code which creates symbolic links in sysfs, pointing to ddc interface used by a particular video output. For example: ls -l /sys/class/drm/card0-HDMI-A-1/ddc lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \ -> ../../../../soc/1388.i2c/i2c-2 This makes it easy for user to associate a display with its ddc adapter and use e.g. ddcutil to control the chosen monitor. This patch adds an i2c_adapter pointer to struct drm_connector. Particular drivers can then use it instead of using their own private instance. If a connector contains a ddc, then create a symbolic link in sysfs. Signed-off-by: Andrzej Pietrasiewicz --- drivers/gpu/drm/drm_sysfs.c | 7 +++ include/drm/drm_connector.h | 11 +++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index ad10810bc972..26d359b39785 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector) /* Let userspace know we have a new connector */ drm_sysfs_hotplug_event(dev); + if (connector->ddc) + return sysfs_create_link(&connector->kdev->kobj, +&connector->ddc->dev.kobj, "ddc"); return 0; } @@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) { if (!connector->kdev) return; + + if (connector->ddc) + sysfs_remove_link(&connector->kdev->kobj, "ddc"); + DRM_DEBUG("removing \"%s\" from sysfs\n", connector->name); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index ca745d9feaf5..1ad3d1d54ba7 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -23,6 +23,7 @@ #ifndef __DRM_CONNECTOR_H__ #define __DRM_CONNECTOR_H__ +#include #include #include #include @@ -1308,6 +1309,16 @@ struct drm_connector { * [0]: progressive, [1]: interlaced */ int audio_latency[2]; + + /** +* @ddc: associated ddc adapter. +* A connector usually has its associated ddc adapter. If a driver uses +* this field, then an appropriate symbolic link is created in connector +* sysfs directory to make it easy for the user to tell which i2c +* adapter is for a particular display. +*/ + struct i2c_adapter *ddc; + /** * @null_edid_counter: track sinks that give us all zeros for the EDID. * Needed to workaround some HW bugs where we get all 0s -- 2.17.1
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #24 from Etienne Lorrain --- The script (attached to this bug) to install latest AMD drivers on ubuntu 19.04 for a RX590 on a Ryzen threadripper works for me, but leads to a Segmentation fault at address 0x0 of /usr/lib/xorg/Xorg seen in .local/share/xorg/Xorg.0.log Solution for me was to: sudo mv /usr/share/X11/xorg.conf.d/10-amdgpu.conf /usr/share/X11/xorg.conf.d/10-amdgpu.conf.bak sudo mv /usr/share/X11/xorg.conf.d/00-amdgpu.conf /usr/share/X11/xorg.conf.d/00-amdgpu.conf.bak as proposed by axlc in https://community.amd.com/thread/227165 Then I had to reboot the PC, no more "loop login". -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107877] deepin-desktop: xdg-email: no method available for opening 'mailto:'
https://bugs.freedesktop.org/show_bug.cgi?id=107877 Routerloginnet changed: What|Removed |Added URL|http://download-alexa.com |https://routerloginnet.tips ||/ --- Comment #27 from Routerloginnet --- What a helpful technical post! I think Wireless Router is the best one to achieve uninterrupted WiFi while blogging. I had some issues with my router which I was unable to fix on my own. Then, I opt for https://routerloginnet.tips/ website which is truly helpful in fixing all my router issues. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] wake_up_var() memory ordering
On Tue, Jun 25, 2019 at 11:19:35AM +0200, Andreas Gruenbacher wrote: > > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c > > index cf4c767005b1..29ea5da7 100644 > > --- a/fs/gfs2/glops.c > > +++ b/fs/gfs2/glops.c > > @@ -227,6 +227,7 @@ static void gfs2_clear_glop_pending(struct gfs2_inode > > *ip) > > return; > > > > clear_bit_unlock(GIF_GLOP_PENDING, &ip->i_flags); > > + smp_mb__after_atomic(); > > wake_up_bit(&ip->i_flags, GIF_GLOP_PENDING); > > This should become clear_and_wake_up_bit as well, right? There are > several more instances of the same pattern. Only if we do as David suggested and make clean_and_wake_up_bit() provide the RELEASE barrier. That is, currently clear_and_wake_up_bit() is clear_bit() smp_mb__after_atomic(); wake_up_bit(); But the above is: clear_bit_unlock(); smp_mb__after_atomic(); wake_up_bit() the difference is that _unlock() uses RELEASE semantics, where clear_bit() does not. The difference is illustrated with something like: cond = true; clear_bit() smp_mb__after_atomic(); wake_up_bit(); In this case, a remote CPU can first observe the clear_bit() and then the 'cond = true' store. When we use clear_bit_unlock() this is not possible, because the RELEASE barrier ensures that everything before, stays before. > > } > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: Attach to DSI host at probe time
On 24.04.2019 15:22, Matt Redfearn wrote: > In contrast to all of the DSI panel drivers in drivers/gpu/drm/panel > which attach to the DSI host via mipi_dsi_attach() at probe time, the > ADV7533 bridge device does not. Instead it defers this to the point that > the upstream device connects to its bridge via drm_bridge_attach(). > The generic Synopsys MIPI DSI host driver does not register it's own > drm_bridge until the MIPI DSI has attached. But it does not call > drm_bridge_attach() on the downstream device until the upstream device > has attached. This leads to a chicken and the egg failure and the DRM > pipeline does not complete. > Since all other mipi_dsi_device drivers call mipi_dsi_attach() in > probe(), make the adv7533 mipi_dsi_device do the same. This ensures that > the Synopsys MIPI DSI host registers it's bridge such that it is > available for the upstream device to connect to. > > Signed-off-by: Matt Redfearn > > --- > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index e7ddd3e3db9..ea36ac3a3de 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -874,9 +874,6 @@ static int adv7511_bridge_attach(struct drm_bridge > *bridge) >&adv7511_connector_helper_funcs); > drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > - if (adv->type == ADV7533) > - ret = adv7533_attach_dsi(adv); > - > if (adv->i2c_main->irq) > regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0), >ADV7511_INT0_HPD); > @@ -1222,7 +1219,11 @@ static int adv7511_probe(struct i2c_client *i2c, const > struct i2c_device_id *id) > drm_bridge_add(&adv7511->bridge); > > adv7511_audio_init(dev, adv7511); > - return 0; > + > + if (adv7511->type == ADV7533) > + return adv7533_attach_dsi(adv7511); > + else > + return 0; It seems that on failure of adv7533_attach_dsi cleanup is not performed. Beside this change looks OK, but it would be good to test it on platforms with adv7533. Regards Andrzej > > err_unregister_cec: > i2c_unregister_device(adv7511->i2c_cec);
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #25 from Etienne Lorrain --- (In reply to Etienne Lorrain from comment #24) > The script (attached to this bug) to install latest AMD drivers on ubuntu > 19.04 for a RX590 on a Ryzen threadripper works for me, but leads to a > Segmentation fault at address 0x0 of /usr/lib/xorg/Xorg seen in > .local/share/xorg/Xorg.0.log > > Solution for me was to: > sudo mv /usr/share/X11/xorg.conf.d/10-amdgpu.conf > /usr/share/X11/xorg.conf.d/10-amdgpu.conf.bak > sudo mv /usr/share/X11/xorg.conf.d/00-amdgpu.conf > /usr/share/X11/xorg.conf.d/00-amdgpu.conf.bak > as proposed by axlc in https://community.amd.com/thread/227165 > Then I had to reboot the PC, no more "loop login". A shorter solution is to comment out (by adding # as first char) the line: ModulePath "/opt/amdgpu/lib/xorg/modules" in 00-amdgpu.conf and rename both files as their original names. Maybe to use /usr/lib/xorg/modules/libglamoregl.so instead of /opt/amdgpu/lib/xorg/modules/libglamoregl.so, different files with same names. Or to use /usr/lib/xorg/modules/drivers/amdgpu_drv.so instead of /opt/amdgpu/lib/xorg/modules/drivers/amdgpu_drv.so, different files with same names. There is only two real files in /opt/amdgpu/lib/xorg/modules/. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] wake_up_var() memory ordering
On Tue, 25 Jun 2019 at 12:36, Peter Zijlstra wrote: > On Tue, Jun 25, 2019 at 11:19:35AM +0200, Andreas Gruenbacher wrote: > > > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c > > > index cf4c767005b1..29ea5da7 100644 > > > --- a/fs/gfs2/glops.c > > > +++ b/fs/gfs2/glops.c > > > @@ -227,6 +227,7 @@ static void gfs2_clear_glop_pending(struct gfs2_inode > > > *ip) > > > return; > > > > > > clear_bit_unlock(GIF_GLOP_PENDING, &ip->i_flags); > > > + smp_mb__after_atomic(); > > > wake_up_bit(&ip->i_flags, GIF_GLOP_PENDING); > > > > This should become clear_and_wake_up_bit as well, right? There are > > several more instances of the same pattern. > > Only if we do as David suggested and make clean_and_wake_up_bit() > provide the RELEASE barrier. (It's clear_and_wake_up_bit, not clean_and_wake_up_bit.) > That is, currently clear_and_wake_up_bit() is > > clear_bit() > smp_mb__after_atomic(); > wake_up_bit(); > > But the above is: > > clear_bit_unlock(); > smp_mb__after_atomic(); > wake_up_bit() > > the difference is that _unlock() uses RELEASE semantics, where > clear_bit() does not. > > The difference is illustrated with something like: > > cond = true; > clear_bit() > smp_mb__after_atomic(); > wake_up_bit(); > > In this case, a remote CPU can first observe the clear_bit() and then > the 'cond = true' store. When we use clear_bit_unlock() this is not > possible, because the RELEASE barrier ensures that everything before, > stays before. Now I'm confused because clear_and_wake_up_bit() in mainline does use clear_bit_unlock(), so it's the exact opposite of what you just said. Thanks, Andreas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110994] [vega10] *ERROR* Failed to initialize parser -125! , running libreoffice
https://bugs.freedesktop.org/show_bug.cgi?id=110994 Bug ID: 110994 Summary: [vega10] *ERROR* Failed to initialize parser -125! , running libreoffice Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: tomenglun...@gmail.com when running libreoffice 6.3.0.0beta1-1 everything freezes and shows a blurred screen. and this starts spamming in the journal. jun 25 14:25:30 tom-pc kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! jun 25 14:25:30 tom-pc kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! jun 25 14:25:30 tom-pc sx[662]: amdgpu: The CS has been cancelled because the context is lost. jun 25 14:25:30 tom-pc sx[662]: amdgpu: The CS has been cancelled because the context is lost. jun 25 14:25:30 tom-pc kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! jun 25 14:25:30 tom-pc kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! jun 25 14:25:31 tom-pc sx[662]: amdgpu: The CS has been cancelled because the context is lost. jun 25 14:25:31 tom-pc sx[662]: amdgpu: The CS has been cancelled because the context is lost. jun 25 14:25:31 tom-pc kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! jun 25 14:25:31 tom-pc kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! jun 25 14:25:31 tom-pc sx[662]: amdgpu: The CS has been cancelled because the context is lost. jun 25 14:25:31 tom-pc sx[662]: amdgpu: The CS has been cancelled because the context is lost. sx is the bash script launching my window manager. instead of "startx" and i have its output piped into systemd journal. systeminfo: libdrm-git 2.4.98.r33.g08bd098d-1 xf86-video-amdgpu-git 19.0.1.5-1 llvm-git 9.0.0_r319552.a4876282704-1 mesa-git 19.2.0_devel.112106.188dbb1679b-1 linux-zen 5.1.12.zen1-1 amd vega 56 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel