Hello Christian,
> Hi,
>
> I assume you use the 1000 MHz firmware. This does also not work on my Rev 7
> board. But I'm pretty sure this is not a problem of the patches, because if
> I take a newer kernel (4.19.20/27) without the patches it also does not
> work. A kernel 4.19.17 does work for me. My opinion on that is that this is
> another problem which does just occure now because now the cpu frequency
> scaling is working with the right frequencies.
I am not sure which firmware i am running, i did all my tests on 5.0.0 and
changing between governors worked fine without the patches
Regards
/Ilias
>
> Ilias Apalodimas <ilias.apalodimas(a)linaro.org> schrieb am Do., 14. März
> 2019, 13:15:
>
> > Hi Gregory,
> > > The clock parenting was not setup properly when DVFS was enabled. It was
> > > expected that the same clock source was used with and without DVFS which
> > > was not the case.
> > >
> > > This patch fixes this issue, allowing to make the cpufreq support work
> > > when the CPU clocks source are not the default ones.
> > >
> > > Fixes: 92ce45fb875d ("cpufreq: Add DVFS support for Armada 37xx")
> > > Cc: <stable(a)vger.kernel.org>
> > > Reported-by: Christian Neubert <christian.neubert.86(a)gmail.com>
> > > Reported-by: Ilias Apalodimas <ilias.apalodimas(a)linaro.org>
> > > Signed-off-by: Gregory CLEMENT <gregory.clement(a)bootlin.com>
> > > ---
> > > drivers/clk/mvebu/armada-37xx-periph.c | 11 +++++++++++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/clk/mvebu/armada-37xx-periph.c
> > b/drivers/clk/mvebu/armada-37xx-periph.c
> > > index 1f1cff428d78..26ed3c18a239 100644
> > > --- a/drivers/clk/mvebu/armada-37xx-periph.c
> > > +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> > > @@ -671,6 +671,17 @@ static int armada_3700_add_composite_clk(const
> > struct clk_periph_data *data,
> > > map = syscon_regmap_lookup_by_compatible(
> > > "marvell,armada-3700-nb-pm");
> > > pmcpu_clk->nb_pm_base = map;
> > > +
> > > + /*
> > > + * Use the same parent when DVFS is enabled that the
> > > + * default parent received at boot time. When this
> > > + * function is called, DVFS is not enabled yet, so we
> > > + * get the default parent and we can set the parent
> > > + * for DVFS.
> > > + */
> > > + if (clk_pm_cpu_set_parent(muxrate_hw,
> > > +
> > clk_pm_cpu_get_parent(muxrate_hw)))
> > > + dev_warn(dev, "Failed to setup default parent
> > clock for DVFS\n");
> > > }
> > >
> > > *hw = clk_hw_register_composite(dev, data->name,
> > data->parent_names,
> > > --
> > > 2.20.1
> > >
> > Applied this and selected only
> >
> > CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE=y
> > CONFIG_CPU_FREQ_GOV_PERFORMANCE=y
> > CONFIG_CPU_FREQ_GOV_POWERSAVE=y
> >
> > After changing the governor from 'powersave' to 'performance' the board
> > completely froze (i even lost access to the serial port)
> >
> > Cheers
> > /Ilias
> >
fuse_dev_splice_write() reads pipe->buffers to determine the size of
'bufs' array before taking the pipe_lock(). This is not safe as
another thread might change the 'pipe->buffers' between the allocation
and taking the pipe_lock(). So we end up with too small 'bufs' array.
Move the bufs allocations inside pipe_lock()/pipe_unlock() to fix this.
Fixes: dd3bb14f44a6 ("fuse: support splice() writing to fuse device")
Signed-off-by: Andrey Ryabinin <aryabinin(a)virtuozzo.com>
Cc: <stable(a)vger.kernel.org>
---
fs/fuse/dev.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index c6b88fa85e2e..702592cce546 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1944,12 +1944,15 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
if (!fud)
return -EPERM;
+ pipe_lock(pipe);
+
bufs = kmalloc_array(pipe->buffers, sizeof(struct pipe_buffer),
GFP_KERNEL);
- if (!bufs)
+ if (!bufs) {
+ pipe_unlock(pipe);
return -ENOMEM;
+ }
- pipe_lock(pipe);
nbuf = 0;
rem = 0;
for (idx = 0; idx < pipe->nrbufs && rem < len; idx++)
--
2.16.4
In the case of a normal sync update, the preparation of framebuffers (be
it calling drm_atomic_helper_prepare_planes() or doing setups with
drm_framebuffer_get()) are performed in the new_state and the respective
cleanups are performed in the old_state.
In the case of async updates, the preparation is also done in the
new_state but the cleanups are done in the new_state (because updates
are performed in place, i.e. in the current state).
The current code blocks async udpates when the fb is changed, turning
async updates into sync updates, slowing down cursor updates and
introducing regressions in igt tests with errors of type:
"CRITICAL: completed 97 cursor updated in a period of 30 flips, we
expect to complete approximately 15360 updates, with the threshold set
at 7680"
Fb changes in async updates were prevented to avoid the following scenario:
- Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
- Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
- Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 (wrong)
Where we have a single call to prepare fb2 but double cleanup call to fb2.
To solve the above problems, instead of blocking async fb changes, we
place the old framebuffer in the new_state object, so when the code
performs cleanups in the new_state it will cleanup the old_fb and we
will have the following scenario instead:
- Async update, oldfb = NULL, newfb = fb1, prepare fb1, no cleanup
- Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb1
- Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2
Where calls to prepare/cleanup are balanced.
Cc: <stable(a)vger.kernel.org> # v4.14+
Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates")
Suggested-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Signed-off-by: Helen Koike <helen.koike(a)collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas(a)amd.com>
---
Hello,
I added a TODO in drm_atomic_helper_async_commit() regarding doing a
full state swap(), Boris and Nicholas, let me know if this is ok and if
I can keep your Reviewed-by tags)
As mentioned in the cover letter, I tested in almost all platforms with
igt plane_cursor_legacy and kms_cursor_legacy and I didn't see any
regressions. But I couldn't test on MSM and AMD because I don't have
the hardware I would appreciate if anyone could help me testing those.
Thanks!
Helen
Changes in v3:
- Add Reviewed-by tags
- Add TODO in drm_atomic_helper_async_commit()
Changes in v2:
- Change the order of the patch in the series, add this as the last one.
- Add documentation
- s/ballanced/balanced
drivers/gpu/drm/drm_atomic_helper.c | 22 ++++++++++++----------
include/drm/drm_modeset_helper_vtables.h | 5 +++++
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2453678d1186..de5812c362b5 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1608,15 +1608,6 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
old_plane_state->crtc != new_plane_state->crtc)
return -EINVAL;
- /*
- * FIXME: Since prepare_fb and cleanup_fb are always called on
- * the new_plane_state for async updates we need to block framebuffer
- * changes. This prevents use of a fb that's been cleaned up and
- * double cleanups from occuring.
- */
- if (old_plane_state->fb != new_plane_state->fb)
- return -EINVAL;
-
funcs = plane->helper_private;
if (!funcs->atomic_async_update)
return -EINVAL;
@@ -1647,6 +1638,8 @@ EXPORT_SYMBOL(drm_atomic_helper_async_check);
* drm_atomic_async_check() succeeds. Async commits are not supposed to swap
* the states like normal sync commits, but just do in-place changes on the
* current state.
+ *
+ * TODO: Implement full swap instead of doing in-place changes.
*/
void drm_atomic_helper_async_commit(struct drm_device *dev,
struct drm_atomic_state *state)
@@ -1657,6 +1650,9 @@ void drm_atomic_helper_async_commit(struct drm_device *dev,
int i;
for_each_new_plane_in_state(state, plane, plane_state, i) {
+ struct drm_framebuffer *new_fb = plane_state->fb;
+ struct drm_framebuffer *old_fb = plane->state->fb;
+
funcs = plane->helper_private;
funcs->atomic_async_update(plane, plane_state);
@@ -1665,11 +1661,17 @@ void drm_atomic_helper_async_commit(struct drm_device *dev,
* plane->state in-place, make sure at least common
* properties have been properly updated.
*/
- WARN_ON_ONCE(plane->state->fb != plane_state->fb);
+ WARN_ON_ONCE(plane->state->fb != new_fb);
WARN_ON_ONCE(plane->state->crtc_x != plane_state->crtc_x);
WARN_ON_ONCE(plane->state->crtc_y != plane_state->crtc_y);
WARN_ON_ONCE(plane->state->src_x != plane_state->src_x);
WARN_ON_ONCE(plane->state->src_y != plane_state->src_y);
+
+ /*
+ * Make sure the FBs have been swapped so that cleanups in the
+ * new_state performs a cleanup in the old FB.
+ */
+ WARN_ON_ONCE(plane_state->fb != old_fb);
}
}
EXPORT_SYMBOL(drm_atomic_helper_async_commit);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index cfb7be40bed7..ce582e8e8f2f 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -1174,6 +1174,11 @@ struct drm_plane_helper_funcs {
* current one with the new plane configurations in the new
* plane_state.
*
+ * Drivers should also swap the framebuffers between plane state
+ * and new_state. This is required because prepare and cleanup calls
+ * are performed on the new_state object, then to cleanup the old
+ * framebuffer, it needs to be placed inside the new_state object.
+ *
* FIXME:
* - It only works for single plane updates
* - Async Pageflips are not supported yet
--
2.20.1
Async update callbacks are expected to set the old_fb in the new_state
so prepare/cleanup framebuffers are balanced.
Cc: <stable(a)vger.kernel.org> # v4.14+
Fixes: 224a4c970987 ("drm/msm: update cursors asynchronously through atomic")
Suggested-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Signed-off-by: Helen Koike <helen.koike(a)collabora.com>
---
Hello,
As mentioned in the cover letter,
But I couldn't test on MSM because I don't have the hardware and I would
appreciate if anyone could test it.
In other platforms (VC4, AMD, Rockchip), there is a hidden
drm_framebuffer_get(new_fb)/drm_framebuffer_put(old_fb) in async_update
that is wrong, but I couldn't identify those here, not sure if it is hidden
somewhere else, but if tests fail this is probably the cause.
Thanks!
Helen
Changes in v3: None
Changes in v2:
- update CC stable and Fixes tag
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index be13140967b4..b854f471e9e5 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -502,6 +502,8 @@ static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
static void mdp5_plane_atomic_async_update(struct drm_plane *plane,
struct drm_plane_state *new_state)
{
+ struct drm_framebuffer *old_fb = plane->state->fb;
+
plane->state->src_x = new_state->src_x;
plane->state->src_y = new_state->src_y;
plane->state->crtc_x = new_state->crtc_x;
@@ -524,6 +526,8 @@ static void mdp5_plane_atomic_async_update(struct drm_plane *plane,
*to_mdp5_plane_state(plane->state) =
*to_mdp5_plane_state(new_state);
+
+ new_state->fb = old_fb;
}
static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = {
--
2.20.1
Some devices come online in write protected state and switch to
read-write once they are ready to process I/O requests. These devices
broke with commit 20bd1d026aac ("scsi: sd: Keep disk read-only when
re-reading partition") because we had no way to distinguish between a
user decision to set a block_device read-only and the actual hardware
device being write-protected.
Because partitions are dropped and recreated on revalidate we are
unable to persist any user-provided policy in hd_struct. Introduce a
bitmap in struct gendisk to track the user configuration. This bitmap
is updated when BLKROSET is called on a given disk or partition.
A helper function, get_user_ro(), is provided to determine whether the
ioctl has forced read-only state for a given block device. This helper
is used by set_disk_ro() and add_partition() to ensure that both
existing and newly created partitions will get the correct state.
- If BLKROSET sets a whole disk device read-only, all partitions will
now end up in a read-only state.
- If BLKROSET sets a given partition read-only, that partition will
remain read-only post revalidate.
- Otherwise both the whole disk device and any partitions will
reflect the write protect state of the underlying device.
Since nobody knows what "policy" means, rename the field to
"read_only" for clarity.
Cc: Jeremy Cline <jeremy(a)jcline.org>
Cc: Oleksii Kurochko <olkuroch(a)cisco.com>
Cc: stable(a)vger.kernel.org # v4.16+
Reported-by: Oleksii Kurochko <olkuroch(a)cisco.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201221
Fixes: 20bd1d026aac ("scsi: sd: Keep disk read-only when re-reading partition")
Signed-off-by: Martin K. Petersen <martin.petersen(a)oracle.com>
---
v3:
- Drop ?: since gcc complains about mixing int and bool (zeroday)
- Drop EXPORT_SYMBOL (hch)
- s/policy/read_only/ and make it a boolean
v2:
- Track user read-only state in a bitmap
- Work around the regression that caused us to drop user
preferences on revalidate
---
block/blk-core.c | 2 +-
block/genhd.c | 34 ++++++++++++++++++++++++----------
block/ioctl.c | 4 ++++
block/partition-generic.c | 7 +++++--
drivers/scsi/sd.c | 4 +---
include/linux/genhd.h | 11 +++++++----
6 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 4673ebe42255..932f179a9095 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -792,7 +792,7 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)
{
const int op = bio_op(bio);
- if (part->policy && op_is_write(op)) {
+ if (part->read_only && op_is_write(op)) {
char b[BDEVNAME_SIZE];
if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
diff --git a/block/genhd.c b/block/genhd.c
index 703267865f14..9f3a93ca1851 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1539,26 +1539,40 @@ static void set_disk_ro_uevent(struct gendisk *gd, int ro)
kobject_uevent_env(&disk_to_dev(gd)->kobj, KOBJ_CHANGE, envp);
}
-void set_device_ro(struct block_device *bdev, int flag)
+void set_device_ro(struct block_device *bdev, bool state)
{
- bdev->bd_part->policy = flag;
+ bdev->bd_part->read_only = state;
}
EXPORT_SYMBOL(set_device_ro);
-void set_disk_ro(struct gendisk *disk, int flag)
+bool get_user_ro(struct gendisk *disk, unsigned int partno)
+{
+ /* Is the user read-only bit set for the whole disk device? */
+ if (test_bit(0, disk->user_ro_bitmap))
+ return true;
+
+ /* Is the user read-only bit set for this particular partition? */
+ if (test_bit(partno, disk->user_ro_bitmap))
+ return true;
+
+ return false;
+}
+
+void set_disk_ro(struct gendisk *disk, bool state)
{
struct disk_part_iter piter;
struct hd_struct *part;
- if (disk->part0.policy != flag) {
- set_disk_ro_uevent(disk, flag);
- disk->part0.policy = flag;
- }
+ if (disk->part0.read_only != state)
+ set_disk_ro_uevent(disk, state);
- disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY);
+ disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY_PART0);
while ((part = disk_part_iter_next(&piter)))
- part->policy = flag;
+ if (get_user_ro(disk, part->partno))
+ part->read_only = true;
+ else
+ part->read_only = state;
disk_part_iter_exit(&piter);
}
@@ -1568,7 +1582,7 @@ int bdev_read_only(struct block_device *bdev)
{
if (!bdev)
return 0;
- return bdev->bd_part->policy;
+ return bdev->bd_part->read_only;
}
EXPORT_SYMBOL(bdev_read_only);
diff --git a/block/ioctl.c b/block/ioctl.c
index 4825c78a6baa..41206df89485 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -451,6 +451,10 @@ static int blkdev_roset(struct block_device *bdev, fmode_t mode,
return ret;
if (get_user(n, (int __user *)arg))
return -EFAULT;
+ if (n)
+ set_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
+ else
+ clear_bit(bdev->bd_partno, bdev->bd_disk->user_ro_bitmap);
set_device_ro(bdev, n);
return 0;
}
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 8e596a8dff32..2bade849cc5c 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -98,7 +98,7 @@ static ssize_t part_ro_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct hd_struct *p = dev_to_part(dev);
- return sprintf(buf, "%d\n", p->policy ? 1 : 0);
+ return sprintf(buf, "%u\n", p->read_only ? 1 : 0);
}
static ssize_t part_alignment_offset_show(struct device *dev,
@@ -338,7 +338,10 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno,
queue_limit_discard_alignment(&disk->queue->limits, start);
p->nr_sects = len;
p->partno = partno;
- p->policy = get_disk_ro(disk);
+ if (get_user_ro(disk, partno))
+ p->read_only = true;
+ else
+ p->read_only = get_disk_ro(disk);
if (info) {
struct partition_meta_info *pinfo = alloc_part_info(disk);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 251db30d0882..9d8e15d03d2b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2608,10 +2608,8 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
int res;
struct scsi_device *sdp = sdkp->device;
struct scsi_mode_data data;
- int disk_ro = get_disk_ro(sdkp->disk);
int old_wp = sdkp->write_prot;
- set_disk_ro(sdkp->disk, 0);
if (sdp->skip_ms_page_3f) {
sd_first_printk(KERN_NOTICE, sdkp, "Assuming Write Enabled\n");
return;
@@ -2649,7 +2647,7 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
"Test WP failed, assume Write Enabled\n");
} else {
sdkp->write_prot = ((data.device_specific & 0x80) != 0);
- set_disk_ro(sdkp->disk, sdkp->write_prot || disk_ro);
+ set_disk_ro(sdkp->disk, sdkp->write_prot);
if (sdkp->first_scan || old_wp != sdkp->write_prot) {
sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
sdkp->write_prot ? "on" : "off");
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 06c0fd594097..1abde0e88ccb 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -118,7 +118,8 @@ struct hd_struct {
unsigned int discard_alignment;
struct device __dev;
struct kobject *holder_dir;
- int policy, partno;
+ bool read_only;
+ int partno;
struct partition_meta_info *info;
#ifdef CONFIG_FAIL_MAKE_REQUEST
int make_it_fail;
@@ -194,6 +195,7 @@ struct gendisk {
*/
struct disk_part_tbl __rcu *part_tbl;
struct hd_struct part0;
+ DECLARE_BITMAP(user_ro_bitmap, DISK_MAX_PARTS);
const struct block_device_operations *fops;
struct request_queue *queue;
@@ -431,12 +433,13 @@ extern void del_gendisk(struct gendisk *gp);
extern struct gendisk *get_gendisk(dev_t dev, int *partno);
extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
-extern void set_device_ro(struct block_device *bdev, int flag);
-extern void set_disk_ro(struct gendisk *disk, int flag);
+extern void set_device_ro(struct block_device *bdev, bool state);
+extern void set_disk_ro(struct gendisk *disk, bool state);
+extern bool get_user_ro(struct gendisk *disk, unsigned int partno);
static inline int get_disk_ro(struct gendisk *disk)
{
- return disk->part0.policy;
+ return disk->part0.read_only;
}
extern void disk_block_events(struct gendisk *disk);
--
2.21.0
The poll condition should only check response_length,
because reads should only be issued if there is data to read.
The response_read flag only prevents double writes.
The problem was that the write set the response_read to false,
enqued a tpm job, and returned. Then application called poll
which checked the response_read flag and returned EPOLLIN.
Then the application called read, but got nothing.
After all that the async_work kicked in.
Added also mutex_lock around the poll check to prevent
other possible race conditions.
Cc: stable(a)vger.kernel.org
Fixes: 9488585b21bef0df12 ("tpm: add support for partial reads")
Reported-by: Mantas Mikulėnas <grawity(a)gmail.com>
Tested-by: Mantas Mikulėnas <grawity(a)gmail.com>
Signed-off-by: Tadeusz Struk <tadeusz.struk(a)intel.com>
---
drivers/char/tpm/tpm-dev-common.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 5eecad233ea1..744b0237300a 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -203,12 +203,19 @@ __poll_t tpm_common_poll(struct file *file, poll_table *wait)
__poll_t mask = 0;
poll_wait(file, &priv->async_wait, wait);
+ mutex_lock(&priv->buffer_mutex);
- if (!priv->response_read || priv->response_length)
+ /*
+ * The response_length indicates if there is still response
+ * (or part of it) to be consumed. Partial reads decrease it
+ * by the number of bytes read, and write resets it the zero.
+ */
+ if (priv->response_length)
mask = EPOLLIN | EPOLLRDNORM;
else
mask = EPOLLOUT | EPOLLWRNORM;
+ mutex_unlock(&priv->buffer_mutex);
return mask;
}