search for: config_read_bitmap

...-- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -578,10 +578,14 @@ static int init_vqs(struct virtio_balloon *vb) static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) { if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, - &vb->config_read_bitmap)) + &vb->config_read_bitmap)) { virtio_cread(vb->vdev, struct virtio_balloon_config, free_page_hint_cmd_id, &vb->cmd_id_received_cache); + /* Legacy balloon config space is LE, unlike all other devices. */ + if (!virtio_has_feature(vb->vdev, VIRTI...

Since virtio-ccw doesn't work with accessing to the config space inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. v1->v2 ChangeLog: - add config_read_bitmap to indicate to the workqueue callbacks about the necessity of reading the related config fields. Wei Wang (2): virtio-balloon: tweak config_changed implementation virtio-balloon: improve update_balloon_size_func drivers/virtio/virtio_balloon.c | 86 +++++++++++++++++++++++++++----------...

virtio-ccw has deadlock issues with reading the config space inside the interrupt context, so we tweak the virtballoon_changed implementation by moving the config read operations into the related workqueue contexts. The config_read_bitmap is used as a flag to the workqueue callbacks about the related config fields that need to be read. Reported-by: Christian Borntraeger <borntraeger at de.ibm.com> Signed-off-by: Wei Wang <wei.w.wang at intel.com> --- drivers/virtio/virtio_balloon.c | 81 +++++++++++++++++++++++++++-----...

On Fri, Jan 04, 2019 at 03:11:52PM +0800, Wei Wang wrote: > virtio-ccw has deadlock issues with reading the config space inside the > interrupt context, so we tweak the virtballoon_changed implementation > by moving the config read operations into the related workqueue contexts. > The config_read_bitmap is used as a flag to the workqueue callbacks > about the related config fields that need to be read. > > Reported-by: Christian Borntraeger <borntraeger at de.ibm.com> > Signed-off-by: Wei Wang <wei.w.wang at intel.com> > --- > drivers/virtio/virtio_balloon.c | 81 ++...

On Fri, Jan 04, 2019 at 03:11:52PM +0800, Wei Wang wrote: > virtio-ccw has deadlock issues with reading the config space inside the > interrupt context, so we tweak the virtballoon_changed implementation > by moving the config read operations into the related workqueue contexts. > The config_read_bitmap is used as a flag to the workqueue callbacks > about the related config fields that need to be read. > > Reported-by: Christian Borntraeger <borntraeger at de.ibm.com> > Signed-off-by: Wei Wang <wei.w.wang at intel.com> > --- > drivers/virtio/virtio_balloon.c | 81 ++...

...e, and have call sites read the latest value via virtio_balloon_cmd_id_received. (Still kept Cornelia and Halil's reviewed-by as it's a minor change) - remove zeroing vb->num_free_page_blocks in probe since vb is allocated via kzalloc. v1->v2 ChangeLog: - add config_read_bitmap to indicate to the workqueue callbacks about the necessity of reading the related config fields. Wei Wang (3): virtio-balloon: tweak config_changed implementation virtio-balloon: improve update_balloon_size_func virtio_balloon: remove the unnecessary 0-initialization drivers/virtio/v...

...o_balloon.c index 0ef16566c3f3..95d9c2f0a7be 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -580,7 +580,7 @@ static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, &vb->config_read_bitmap)) virtio_cread(vb->vdev, struct virtio_balloon_config, - free_page_report_cmd_id, + free_page_hint_cmd_id, &vb->cmd_id_received_cache); return vb->cmd_id_received_cache; diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h...

...o_balloon.c index 0ef16566c3f3..95d9c2f0a7be 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -580,7 +580,7 @@ static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, &vb->config_read_bitmap)) virtio_cread(vb->vdev, struct virtio_balloon_config, - free_page_report_cmd_id, + free_page_hint_cmd_id, &vb->cmd_id_received_cache); return vb->cmd_id_received_cache; diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h...

...o_balloon.c index 0ef16566c3f3..95d9c2f0a7be 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -580,7 +580,7 @@ static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, &vb->config_read_bitmap)) virtio_cread(vb->vdev, struct virtio_balloon_config, - free_page_report_cmd_id, + free_page_hint_cmd_id, &vb->cmd_id_received_cache); return vb->cmd_id_received_cache; diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h...

...correctly the command id is actually assumed native endian: > > > > > > static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > > { > > if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > > &vb->config_read_bitmap)) > > virtio_cread(vb->vdev, struct virtio_balloon_config, > > free_page_hint_cmd_id, > > &vb->cmd_id_received_cache); > > > > return vb->cmd_id_received_cache; > > } &...

...correctly the command id is actually assumed native endian: > > > > > > static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > > { > > if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > > &vb->config_read_bitmap)) > > virtio_cread(vb->vdev, struct virtio_balloon_config, > > free_page_hint_cmd_id, > > &vb->cmd_id_received_cache); > > > > return vb->cmd_id_received_cache; > > } &...

...gh, and host only cares about 0 or non 0 ATM. As you say correctly the command id is actually assumed native endian: static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) { if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, &vb->config_read_bitmap)) virtio_cread(vb->vdev, struct virtio_balloon_config, free_page_hint_cmd_id, &vb->cmd_id_received_cache); return vb->cmd_id_received_cache; } So guest assumes native, host assumes LE. > > --...

...gh, and host only cares about 0 or non 0 ATM. As you say correctly the command id is actually assumed native endian: static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) { if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, &vb->config_read_bitmap)) virtio_cread(vb->vdev, struct virtio_balloon_config, free_page_hint_cmd_id, &vb->cmd_id_received_cache); return vb->cmd_id_received_cache; } So guest assumes native, host assumes LE. > > --...

...@@ -77,6 +81,8 @@ struct virtio_balloon { > > /* Prevent updating balloon when it is being canceled. */ > > spinlock_t stop_update_lock; > > bool stop_update; > > + /* Bitmap to indicate if reading the related config fields are needed > */ > > + unsigned long config_read_bitmap; > > > > /* The list of allocated free pages, waiting to be given back to mm */ > > struct list_head free_page_list; > > It seems that you never initialize this bitmap. Probably harmless here but > generally using uninitialized memory isn't good. We've used...

...6c3f3..95d9c2f0a7be 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -580,7 +580,7 @@ static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > &vb->config_read_bitmap)) > virtio_cread(vb->vdev, struct virtio_balloon_config, > - free_page_report_cmd_id, > + free_page_hint_cmd_id, > &vb->cmd_id_received_cache); > > return vb->cmd_id_received_cache; > diff --git a/include/uapi/linux/virtio_balloon.h b/...

...ct virtio_balloon_config, actual, + &actual); } static void update_balloon_stats_func(struct work_struct *work) @@ -579,12 +573,10 @@ static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) { if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, &vb->config_read_bitmap)) { - virtio_cread(vb->vdev, struct virtio_balloon_config, - free_page_hint_cmd_id, - &vb->cmd_id_received_cache); /* Legacy balloon config space is LE, unlike all other devices. */ - if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1)) - vb->cmd_id_received_c...

...tually assumed native endian: > > > > > > > > > static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > > > { > > > if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > > > &vb->config_read_bitmap)) > > > virtio_cread(vb->vdev, struct virtio_balloon_config, > > > free_page_hint_cmd_id, > > > &vb->cmd_id_received_cache); > > > > > > return vb->cmd_id_rece...

...to do that. > As you say correctly the command id is actually assumed native endian: > > > static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > { > if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > &vb->config_read_bitmap)) > virtio_cread(vb->vdev, struct virtio_balloon_config, > free_page_hint_cmd_id, > &vb->cmd_id_received_cache); > > return vb->cmd_id_received_cache; > } > > > So guest assumes...

Page reporting features were never supported by legacy hypervisors. Supporting them poses a problem: should we use native endian-ness (like current code assumes)? Or little endian-ness like the virtio spec says? Rather than try to figure out, and since results of incorrect endian-ness are dire, let's just block this configuration. Cc: stable at vger.kernel.org Signed-off-by: Michael S.

Page reporting features were never supported by legacy hypervisors. Supporting them poses a problem: should we use native endian-ness (like current code assumes)? Or little endian-ness like the virtio spec says? Rather than try to figure out, and since results of incorrect endian-ness are dire, let's just block this configuration. Cc: stable at vger.kernel.org Signed-off-by: Michael S.