Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-22 Thread michael . christie
On 10/22/21 11:12 AM, michael.chris...@oracle.com wrote:
> On 10/22/21 5:47 AM, Michael S. Tsirkin wrote:
>>> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
>>> index c998860d7bbc..e5c0669430e5 100644
>>> --- a/include/uapi/linux/vhost.h
>>> +++ b/include/uapi/linux/vhost.h
>>> @@ -70,6 +70,17 @@
>>>  #define VHOST_VRING_BIG_ENDIAN 1
>>>  #define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct 
>>> vhost_vring_state)
>>>  #define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct 
>>> vhost_vring_state)
>>> +/* By default, a device gets one vhost_worker created during 
>>> VHOST_SET_OWNER
>>> + * that its virtqueues share. This allows userspace to create a 
>>> vhost_worker
>>> + * and map a virtqueue to it or map a virtqueue to an existing worker.
>>> + *
>>> + * If pid > 0 and it matches an existing vhost_worker thread it will be 
>>> bound
>>> + * to the vq. If pid is VHOST_VRING_NEW_WORKER, then a new worker will be
>>> + * created and bound to the vq.
>>> + *
>>> + * This must be called after VHOST_SET_OWNER and before the vq is active.
>>> + */
>>
>> A couple of things here:
>> it's probably a good idea not to make it match pid exactly,
>> if for no other reason than I'm not sure we want to
>> commit this being a pid. Let's just call it an id?
> 
> Ok.
> 
>> And maybe byteswap it or xor with some value
>> just to make sure userspace does not begin abusing it anyway.
>>
>> Also, interaction with pid namespace is unclear to me.
>> Can you document what happens here?
> 
> This current patchset only allows the vhost_dev owner to
> create/bind workers for devices it owns, so namespace don't come

I made a mistake here. The patches do restrict VHOST_SET_VRING_WORKER
to the same owner like I wrote. However, it looks like we could have 2
threads with the same mm pointer so vhost_dev_check_owner returns true,
but they could be in different namespaces.

Even though we are not going to pass the pid_t between user/kernel
space, should I add a pid namespace check when I repost the patches?



> into play. If a thread from another namespace tried to create/bind
> a worker we would hit the owner checks in vhost_dev_ioctl which is
> done before vhost_vring_ioctl normally (for vdpa we hit the use_worker
> check and fail there).
> 
> However, with the kernel worker API changes the worker threads will
> now be in the vhost dev owner's namespace and not the kthreadd/default
> one, so in the future we are covered if we want to do something more
> advanced. For example, I've seen people working on an API to export the
> worker pids:
> 
> https://lore.kernel.org/netdev/20210507154332.hiblsd6ot5wzwkdj@steredhat/T/
> 
> and in the future for interfaces that export that info we could restrict
> access to root or users from the same namespace or I guess add interfaces
> to allow different namespaces to see the workers and share them.
> 
> 
>> No need to fix funky things like moving the fd between
>> pid namespaces while also creating/destroying workers, but let's
>> document it's not supported.
> 
> Ok. I'll add a comment.
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-22 Thread michael . christie
On 10/22/21 5:47 AM, Michael S. Tsirkin wrote:
>> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
>> index c998860d7bbc..e5c0669430e5 100644
>> --- a/include/uapi/linux/vhost.h
>> +++ b/include/uapi/linux/vhost.h
>> @@ -70,6 +70,17 @@
>>  #define VHOST_VRING_BIG_ENDIAN 1
>>  #define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct 
>> vhost_vring_state)
>>  #define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct 
>> vhost_vring_state)
>> +/* By default, a device gets one vhost_worker created during VHOST_SET_OWNER
>> + * that its virtqueues share. This allows userspace to create a vhost_worker
>> + * and map a virtqueue to it or map a virtqueue to an existing worker.
>> + *
>> + * If pid > 0 and it matches an existing vhost_worker thread it will be 
>> bound
>> + * to the vq. If pid is VHOST_VRING_NEW_WORKER, then a new worker will be
>> + * created and bound to the vq.
>> + *
>> + * This must be called after VHOST_SET_OWNER and before the vq is active.
>> + */
> 
> A couple of things here:
> it's probably a good idea not to make it match pid exactly,
> if for no other reason than I'm not sure we want to
> commit this being a pid. Let's just call it an id?

Ok.

> And maybe byteswap it or xor with some value
> just to make sure userspace does not begin abusing it anyway.
> 
> Also, interaction with pid namespace is unclear to me.
> Can you document what happens here?

This current patchset only allows the vhost_dev owner to
create/bind workers for devices it owns, so namespace don't come
into play. If a thread from another namespace tried to create/bind
a worker we would hit the owner checks in vhost_dev_ioctl which is
done before vhost_vring_ioctl normally (for vdpa we hit the use_worker
check and fail there).

However, with the kernel worker API changes the worker threads will
now be in the vhost dev owner's namespace and not the kthreadd/default
one, so in the future we are covered if we want to do something more
advanced. For example, I've seen people working on an API to export the
worker pids:

https://lore.kernel.org/netdev/20210507154332.hiblsd6ot5wzwkdj@steredhat/T/

and in the future for interfaces that export that info we could restrict
access to root or users from the same namespace or I guess add interfaces
to allow different namespaces to see the workers and share them.


> No need to fix funky things like moving the fd between
> pid namespaces while also creating/destroying workers, but let's
> document it's not supported.

Ok. I'll add a comment.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V3 00/11] vhost: multiple worker support

2021-10-22 Thread michael . christie
Ccing Christian for the kernel worker API merging stuff.

On 10/22/21 4:48 AM, Michael S. Tsirkin wrote:
> On Fri, Oct 22, 2021 at 12:18:59AM -0500, Mike Christie wrote:
>> The following patches apply over linus's tree and this patchset
>>
>> https://urldefense.com/v3/__https://lore.kernel.org/all/20211007214448.6282-1-michael.chris...@oracle.com/__;!!ACWV5N9M2RV99hQ!aqbE06mycEW-AMIj5avlBMDSvg2FONlNdYHr8PcNKdvl5FeO4QLCxCOyaVg8g8C2_Kp5$
>>  
>>
>> which allows us to check the vhost owner thread's RLIMITs:
> 
> 
> Unfortunately that patchset in turn triggers kbuild warnings.

Yeah, that's the Jens/Paul issue I mentioned. I have to remove the
old create_io_thread code and resolve issues with their trees. Paul's
tree has a conflict with Jens and then my patch has a issue with Paul's
patches.

So Christian and I thought we would re-push the patchset through
Christian after that has settled in 5.16-rc1 and then shoot for 5.17
so it has time to bake in next.


> I was hoping you would address them, I don't think
> merging that patchset before kbuild issues are addressed
> is possible.
> 
> It also doesn't have lots of acks, I'm a bit apprehensive
> of merging core changes like this through the vhost tree.

Ok. Just to make sure we are on the same page. Christian was going to
push the kernel worker API changes.

> Try to CC more widely/ping people?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

2021-10-22 Thread Parav Pandit via Virtualization
Hi Michael,

> From: Michael S. Tsirkin 
> Sent: Friday, October 22, 2021 4:11 PM
> 
> On Thu, Oct 21, 2021 at 07:35:01PM +0300, Parav Pandit wrote:
> > Currently user cannot view the vdpa device config space. Also user
> > cannot set the mac address and mtu of the vdpa device.
> > This patchset enables users to set the mac address and mtu of the vdpa
> > device once the device is created.
> > If a vendor driver supports such configuration user can set it
> > otherwise user gets unsupported error.
> >
> > vdpa mac address and mtu are device configuration layout fields.
> > To keep interface generic enough for multiple types of vdpa devices,
> > mac address and mtu setting is implemented as configuration layout fields.
> 
> 
> So I got lots of "sha1 is lacking or useless" warnings with this.
> I did my best to merge but please check out the result in linux-next.
>
I was able to test vdpasim_net on one system with rebase code with these 
patches.
On 2nd system I started verifying mlx5 vdpa.
System has mlx5 vdpa and virtio blk devices.
After rebase, I hit the below crash at system boot time on virtio blk device.

I have some internal issues in accessing crashed system, unable to proceed to 
verify it.
However, the crash doesn't seems to occur due to this patches, as its happening 
on virtio blk dev (non vdpa based blk dev).

[1.238098] virtio_blk virtio0: [vda] 73400320 512-byte logical blocks (37.6 
GB/35.0 GiB)
 [1.240177] BUG: unable to handle page fault for address: fffedbeb
 [1.240973] #PF: supervisor write access in kernel mode
 [1.241590] #PF: error_code(0x0002) - not-present page
 [1.242202] PGD 0 P4D 0 
 [1.242559] Oops: 0002 [#1] SMP NOPTI
 [1.243023] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-vdpa-mac+ 
#7
 [1.243559] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
 [1.243559] RIP: 0010:virtqueue_add_sgs+0x9e2/0xe50
 [1.243559] Code: 0e 41 89 4d 40 49 8b 4d 78 48 89 1c 11 48 8b 5c 24 08 49 
8b 4d 78 48 89 5c 11 08 49 8b 95 b0 00 00 00 48 85 d2 74 06 8b 1c 24 <89> 1c 82 
41 8b 45 50 0f b7 5c 24 10 49 8b 55 60 83 e8 01 66 41 23
 [1.243559] RSP: :88810092b5a8 EFLAGS: 00010006
 [1.243559] RAX:  RBX: 1001 RCX: 
888104f91000
 [1.243559] RDX: fffedbeb RSI:  RDI: 
0004
 [1.243559] RBP: 0003 R08: 0004 R09: 
8289920c
 [1.243559] R10: 0003 R11: 0001 R12: 
0030
 [1.243559] R13: 88810547ef00 R14: 0001 R15: 

 [1.243559] FS:  () GS:88846fa0() 
knlGS:
 [1.243559] CS:  0010 DS:  ES:  CR0: 80050033
 [1.243559] CR2: fffedbeb CR3: 02612001 CR4: 
00370eb0
 [1.243559] DR0:  DR1:  DR2: 

 [1.243559] DR3:  DR6: fffe0ff0 DR7: 
0400
 [1.243559] Call Trace:
 [1.243559]  virtio_queue_rq+0x1e4/0x5f0
 [1.243559]  __blk_mq_try_issue_directly+0x138/0x1e0
 [1.243559]  blk_mq_try_issue_directly+0x47/0xa0
 [1.243559]  blk_mq_submit_bio+0x5af/0x890
 [1.243559]  __submit_bio+0x2af/0x2e0
 [1.243559]  ? create_page_buffers+0x55/0x70
 [1.243559]  submit_bio_noacct+0x26c/0x2d0
 [1.243559]  submit_bh_wbc.isra.0+0x122/0x150
 [1.243559]  block_read_full_page+0x2f7/0x3f0
 [1.243559]  ? blkdev_direct_IO+0x4b0/0x4b0
 [1.243559]  ? lru_cache_add+0xcf/0x150
 [1.243559]  do_read_cache_page+0x4f2/0x6a0
 [1.243559]  ? lockdep_hardirqs_on_prepare+0xe4/0x190
 [1.243559]  read_part_sector+0x39/0xd0
 [1.243559]  read_lba+0xca/0x140
 [1.243559]  efi_partition+0xe6/0x770
 [1.243559]  ? snprintf+0x49/0x60
 [1.243559]  ? is_gpt_valid.part.0+0x420/0x420
 [1.243559]  bdev_disk_changed+0x21c/0x4a0
 [1.243559]  blkdev_get_whole+0x76/0x90
 [1.243559]  blkdev_get_by_dev+0xd4/0x2c0
 [1.243559]  device_add_disk+0x351/0x390
 [1.243559]  virtblk_probe+0x804/0xa40
 [1.243559]  ? ncpus_cmp_func+0x10/0x10
 [1.243559]  virtio_dev_probe+0x155/0x250
 [1.243559]  really_probe+0xdb/0x3b0
 [1.243559]  __driver_probe_device+0x8c/0x180
 [1.243559]  driver_probe_device+0x1e/0xa0
 [1.243559]  __driver_attach+0xaa/0x160
 [1.243559]  ? __device_attach_driver+0x110/0x110
 [1.243559]  ? __device_attach_driver+0x110/0x110
 [1.243559]  bus_for_each_dev+0x7a/0xc0
 [1.243559]  bus_add_driver+0x198/0x200
 [1.243559]  driver_register+0x6c/0xc0
 [1.243559]  ? loop_init+0x149/0x149
 [1.243559]  init+0x52/0x7d
 [1.243559]  do_one_initcall+0x63/0x300
 [1.243559]  ? rcu_read_lock_sched_held+0x5d/0x90
 [1.243559]  kernel_init_freeable+0x26a/0x2b2
 [1.243559]  ? rest_init+0x2e0/0x2e0
 [1.243559]  kernel_init+0x17/0x130
 [   

[PATCH 9/9] drm: Clarify semantics of struct drm_mode_config.{min, max}_{width, height}

2021-10-22 Thread Thomas Zimmermann
Add additional information on the semantics of the size fields in
struct drm_mode_config. Also add a TODO to review all driver for
correct usage of these fields.

Signed-off-by: Thomas Zimmermann 
---
 Documentation/gpu/todo.rst| 15 +++
 include/drm/drm_mode_config.h | 13 +
 2 files changed, 28 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 60d1d7ee0719..f4e1d72149f7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -463,6 +463,21 @@ Contact: Thomas Zimmermann , 
Christian König, Daniel Vette
 
 Level: Intermediate
 
+Review all drivers for setting struct drm_mode_config.{max_width,max_height} 
correctly
+--
+
+The values in struct drm_mode_config.{max_width,max_height} describe the
+maximum supported framebuffer size. It's the virtual screen size, but many
+drivers treat it like limitations of the physical resolution.
+
+The maximum width depends on the hardware's maximum scanline pitch. The
+maximum height depends on the amount of addressable video memory. Review all
+drivers to initialize the fields to the correct values.
+
+Contact: Thomas Zimmermann 
+
+Level: Intermediate
+
 
 Core refactorings
 =
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..91ca575a78de 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -359,6 +359,19 @@ struct drm_mode_config_funcs {
  * Core mode resource tracking structure.  All CRTC, encoders, and connectors
  * enumerated by the driver are added here, as are global properties.  Some
  * global restrictions are also here, e.g. dimension restrictions.
+ *
+ * Framebuffer sizes refer to the virtual screen that can be displayed by
+ * the CRTC. This can be different from the physical resolution programmed.
+ * The minimum width and height, stored in @min_width and @min_height,
+ * describe the smallest size of the framebuffer. It correlates to the
+ * minimum programmable resolution.
+ * The maximum width, stored in @max_width, is typically limited by the
+ * maximum pitch between two adjacent scanlines. The maximum height, stored
+ * in @max_height, is usually only limited by the amount of addressable video
+ * memory. For hardware that has no real maximum, drivers should pick a
+ * reasonable default.
+ *
+ * See also @DRM_SHADOW_PLANE_MAX_WIDTH and @DRM_SHADOW_PLANE_MAX_HEIGHT.
  */
 struct drm_mode_config {
/**
-- 
2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 5/9] drm/format-helper: Streamline blit-helper interface

2021-10-22 Thread Thomas Zimmermann
Move destination-buffer clipping from format-helper blit function into
caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for
consistency with the rest of the interface. Remove drm_fb_blit_dstclip(),
which isn't required.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_format_helper.c | 51 -
 drivers/gpu/drm/tiny/simpledrm.c| 14 +---
 include/drm/drm_format_helper.h | 10 ++
 3 files changed, 19 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c 
b/drivers/gpu/drm/drm_format_helper.c
index 260dc587c1d7..5ca9abc65510 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -463,7 +463,7 @@ void drm_fb_xrgb_to_gray8(void *dst, unsigned int 
dst_pitch, const void *vad
 EXPORT_SYMBOL(drm_fb_xrgb_to_gray8);
 
 /**
- * drm_fb_blit_rect_dstclip - Copy parts of a framebuffer to display memory
+ * drm_fb_blit_toio - Copy parts of a framebuffer to display memory
  * @dst:   The display memory to copy to
  * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @dst_format:FOURCC code of the display's color format
@@ -475,17 +475,14 @@ EXPORT_SYMBOL(drm_fb_xrgb_to_gray8);
  * formats of the display and the framebuffer mismatch, the blit function
  * will attempt to convert between them.
  *
- * Use drm_fb_blit_dstclip() to copy the full framebuffer.
- *
  * Returns:
  * 0 on success, or
  * -EINVAL if the color-format conversion failed, or
  * a negative error code otherwise.
  */
-int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
-uint32_t dst_format, void *vmap,
-struct drm_framebuffer *fb,
-struct drm_rect *clip)
+int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t 
dst_format,
+const void *vmap, const struct drm_framebuffer *fb,
+const struct drm_rect *clip)
 {
uint32_t fb_format = fb->format->format;
 
@@ -496,20 +493,16 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned 
int dst_pitch,
dst_format = DRM_FORMAT_XRGB;
 
if (dst_format == fb_format) {
-   dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
return 0;
 
} else if (dst_format == DRM_FORMAT_RGB565) {
if (fb_format == DRM_FORMAT_XRGB) {
-   dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
-   drm_fb_xrgb_to_rgb565_toio(dst, dst_pitch, vmap, 
fb, clip,
-  false);
+   drm_fb_xrgb_to_rgb565_toio(dst, dst_pitch, vmap, 
fb, clip, false);
return 0;
}
} else if (dst_format == DRM_FORMAT_RGB888) {
if (fb_format == DRM_FORMAT_XRGB) {
-   dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
drm_fb_xrgb_to_rgb888_toio(dst, dst_pitch, vmap, 
fb, clip);
return 0;
}
@@ -517,36 +510,4 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned 
int dst_pitch,
 
return -EINVAL;
 }
-EXPORT_SYMBOL(drm_fb_blit_rect_dstclip);
-
-/**
- * drm_fb_blit_dstclip - Copy framebuffer to display memory
- * @dst:   The display memory to copy to
- * @dst_pitch: Number of bytes between two consecutive scanlines within dst
- * @dst_format:FOURCC code of the display's color format
- * @vmap:  The framebuffer memory to copy from
- * @fb:The framebuffer to copy from
- *
- * This function copies a full framebuffer to display memory. If the formats
- * of the display and the framebuffer mismatch, the copy function will
- * attempt to convert between them.
- *
- * See drm_fb_blit_rect_dstclip() for more information.
- *
- * Returns:
- * 0 on success, or a negative error code otherwise.
- */
-int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
-   uint32_t dst_format, void *vmap,
-   struct drm_framebuffer *fb)
-{
-   struct drm_rect fullscreen = {
-   .x1 = 0,
-   .x2 = fb->width,
-   .y1 = 0,
-   .y2 = fb->height,
-   };
-   return drm_fb_blit_rect_dstclip(dst, dst_pitch, dst_format, vmap, fb,
-   );
-}
-EXPORT_SYMBOL(drm_fb_blit_dstclip);
+EXPORT_SYMBOL(drm_fb_blit_toio);
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 481b48bde047..571f716ff427 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -641,6 +641,8 @@ simpledrm_simple_display_pipe_enable(struct 
drm_simple_display_pipe *pipe,
struct drm_framebuffer *fb = 

[PATCH 8/9] drm/simpledrm: Support virtual screen sizes

2021-10-22 Thread Thomas Zimmermann
Add constants for the maximum size of the shadow-plane surface
size. Useful for shadow planes with virtual screen sizes. The
current sizes are 4096 scanlines with 4096 pixels each. This
seems reasonable for current hardware, but can be increased as
necessary.

In simpledrm, set the maximum framebuffer size from the constants
for shadow planes. Implements support for virtual screen sizes and
page flipping on the fbdev console.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/tiny/simpledrm.c|  9 +++--
 include/drm/drm_gem_atomic_helper.h | 18 ++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index e872121e9fb0..e42ae1c6ebcd 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -776,6 +777,7 @@ static int simpledrm_device_init_modeset(struct 
simpledrm_device *sdev)
struct drm_display_mode *mode = >mode;
struct drm_connector *connector = >connector;
struct drm_simple_display_pipe *pipe = >pipe;
+   unsigned long max_width, max_height;
const uint32_t *formats;
size_t nformats;
int ret;
@@ -784,10 +786,13 @@ static int simpledrm_device_init_modeset(struct 
simpledrm_device *sdev)
if (ret)
return ret;
 
+   max_width = max_t(unsigned long, mode->hdisplay, 
DRM_SHADOW_PLANE_MAX_WIDTH);
+   max_height = max_t(unsigned long, mode->vdisplay, 
DRM_SHADOW_PLANE_MAX_HEIGHT);
+
dev->mode_config.min_width = mode->hdisplay;
-   dev->mode_config.max_width = mode->hdisplay;
+   dev->mode_config.max_width = max_width;
dev->mode_config.min_height = mode->vdisplay;
-   dev->mode_config.max_height = mode->vdisplay;
+   dev->mode_config.max_height = max_height;
dev->mode_config.prefer_shadow_fbdev = true;
dev->mode_config.preferred_depth = sdev->format->cpp[0] * 8;
dev->mode_config.funcs = _mode_config_funcs;
diff --git a/include/drm/drm_gem_atomic_helper.h 
b/include/drm/drm_gem_atomic_helper.h
index 48222a107873..54983ecf641a 100644
--- a/include/drm/drm_gem_atomic_helper.h
+++ b/include/drm/drm_gem_atomic_helper.h
@@ -22,6 +22,24 @@ int drm_gem_simple_display_pipe_prepare_fb(struct 
drm_simple_display_pipe *pipe,
  * Helpers for planes with shadow buffers
  */
 
+/**
+ * DRM_SHADOW_PLANE_MAX_WIDTH - Maximum width of a plane's shadow buffer in 
pixels
+ *
+ * For drivers with shadow planes, the maximum width of the framebuffer is
+ * usually independent from hardware limitations. Drivers can initialize struct
+ * drm_mode_config.max_width from DRM_SHADOW_PLANE_MAX_WIDTH.
+ */
+#define DRM_SHADOW_PLANE_MAX_WIDTH (1ul << 12)
+
+/**
+ * DRM_SHADOW_PLANE_MAX_HEIGHT - Maximum height of a plane's shadow buffer in 
scanlines
+ *
+ * For drivers with shadow planes, the maximum height of the framebuffer is
+ * usually independent from hardware limitations. Drivers can initialize struct
+ * drm_mode_config.max_height from DRM_SHADOW_PLANE_MAX_HEIGHT.
+ */
+#define DRM_SHADOW_PLANE_MAX_HEIGHT(1ul << 12)
+
 /**
  * struct drm_shadow_plane_state - plane state for planes with shadow buffers
  *
-- 
2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 0/9] drm/simpledrm: Enable damage clips and virtual screens

2021-10-22 Thread Thomas Zimmermann
Enable FB_DAMAGE_CLIPS with simpledrm for improved performance and/or
less overhead. With this in place, add support for virtual screens
(i.e., framebuffers that are larger than the display resolution). This
also enables fbdev panning and page flipping.

After the discussion and bug fixing wrt to fbdev overallocation, I
decided to add full support for this to simpledrm. Patches #1 to #5
change the format-helper functions accordingly. Destination buffers
are now clipped by the caller and all functions support a similar
feature set. This has some fallout in various drivers.

Patch #6 change fbdev emulation to support overallocation with
shadow buffers, even if the hardware buffer would be too small.

Patch #7 and #8 update simpledrm to enable damage clipping and virtual
screen sizes. Both feature go hand in hand, sort of. For shadow-
buffered planes, the DRM framebuffer lives in system memory. So the
maximum size of the virtual screen is somewhat arbitrary. We add two
constants for resonable maximum width and height of 4096 each.

Patch #9 adds documentation and a TODO item.

Tested with simpledrm. I also ran the recently posted fbdev panning
tests to make sure that the fbdev overallocation works correctly. [1]

[1] https://lists.freedesktop.org/archives/igt-dev/2021-October/036642.html

Thomas Zimmermann (9):
  drm/format-helper: Export drm_fb_clip_offset()
  drm/format-helper: Rework format-helper memcpy functions
  drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
  drm/format-helper: Rework format-helper conversion functions
  drm/format-helper: Streamline blit-helper interface
  drm/fb-helper: Allocate shadow buffer of surface height
  drm/simpledrm: Enable FB_DAMAGE_CLIPS property
  drm/simpledrm: Support virtual screen sizes
  drm: Clarify semantics of struct
drm_mode_config.{min,max}_{width,height}

 Documentation/gpu/todo.rst  |  15 ++
 drivers/gpu/drm/drm_fb_helper.c |   2 +-
 drivers/gpu/drm/drm_format_helper.c | 236 ++--
 drivers/gpu/drm/drm_mipi_dbi.c  |   6 +-
 drivers/gpu/drm/gud/gud_pipe.c  |  14 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |   5 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c  |   4 +-
 drivers/gpu/drm/tiny/cirrus.c   |  24 +-
 drivers/gpu/drm/tiny/repaper.c  |   2 +-
 drivers/gpu/drm/tiny/simpledrm.c|  41 +++-
 drivers/gpu/drm/tiny/st7586.c   |   2 +-
 include/drm/drm_format_helper.h |  58 ++---
 include/drm/drm_gem_atomic_helper.h |  18 ++
 include/drm/drm_mode_config.h   |  13 ++
 14 files changed, 254 insertions(+), 186 deletions(-)

--
2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 6/9] drm/fb-helper: Allocate shadow buffer of surface height

2021-10-22 Thread Thomas Zimmermann
Allocating a shadow buffer of the height of the buffer object does
not support fbdev overallocation. Use surface height instead.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8e7a124d6c5a..9727a59d35fd 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2338,7 +2338,7 @@ static int drm_fb_helper_generic_probe(struct 
drm_fb_helper *fb_helper,
return PTR_ERR(fbi);
 
fbi->fbops = _fbdev_fb_ops;
-   fbi->screen_size = fb->height * fb->pitches[0];
+   fbi->screen_size = sizes->surface_height * fb->pitches[0];
fbi->fix.smem_len = fbi->screen_size;
 
drm_fb_helper_fill_info(fbi, fb_helper, sizes);
-- 
2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property

2021-10-22 Thread Thomas Zimmermann
Enable the FB_DAMAGE_CLIPS property to reduce display-update
overhead. Also fixes a warning in the kernel log.

  simple-framebuffer simple-framebuffer.0: [drm] 
drm_plane_enable_fb_damage_clips() not called

Fix the computation of the blit rectangle. This wasn't an issue so
far, as simpledrm always blitted the full framebuffer. The code now
supports damage clipping and virtual screen sizes.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/tiny/simpledrm.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 571f716ff427..e872121e9fb0 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct 
drm_simple_display_pipe *pipe,
void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping 
abstraction */
struct drm_device *dev = >dev;
void __iomem *dst = sdev->screen_base;
-   struct drm_rect clip;
+   struct drm_rect src_clip, dst_clip;
int idx;
 
if (!fb)
@@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct 
drm_simple_display_pipe *pipe,
if (!drm_dev_enter(dev, ))
return;
 
-   drm_rect_init(, 0, 0, fb->width, fb->height);
+   drm_rect_fp_to_int(_clip, _state->src);
 
-   dst += drm_fb_clip_offset(sdev->pitch, sdev->format, );
-   drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, 
);
+   dst_clip = plane_state->dst;
+   if (!drm_rect_intersect(_clip, _clip))
+   return;
+
+   dst += drm_fb_clip_offset(sdev->pitch, sdev->format, _clip);
+   drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, 
_clip);
 
drm_dev_exit(idx);
 }
@@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct 
drm_simple_display_pipe *pipe,
struct drm_framebuffer *fb = plane_state->fb;
struct drm_device *dev = >dev;
void __iomem *dst = sdev->screen_base;
-   struct drm_rect clip;
+   struct drm_rect damage_clip, src_clip, dst_clip;
int idx;
 
if (!fb)
return;
 
-   if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, 
))
+   if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, 
_clip))
+   return;
+
+   drm_rect_fp_to_int(_clip, _state->src);
+   if (!drm_rect_intersect(_clip, _clip))
+   return;
+
+   dst_clip = plane_state->dst;
+   if (!drm_rect_intersect(_clip, _clip))
return;
 
if (!drm_dev_enter(dev, ))
return;
 
-   dst += drm_fb_clip_offset(sdev->pitch, sdev->format, );
-   drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, 
);
+   dst += drm_fb_clip_offset(sdev->pitch, sdev->format, _clip);
+   drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, 
_clip);
 
drm_dev_exit(idx);
 }
@@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct 
simpledrm_device *sdev)
if (ret)
return ret;
 
+   drm_plane_enable_fb_damage_clips(>plane);
+
drm_mode_config_reset(dev);
 
return 0;
-- 
2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 4/9] drm/format-helper: Rework format-helper conversion functions

2021-10-22 Thread Thomas Zimmermann
Move destination-buffer clipping from all format-helper conversion
functions into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.

Simply harmonize the interface and semantics of the existing code.
Not all conversion helpers support all combinations of parameters.
We have to add additional features when we need them.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_format_helper.c | 131 +++-
 drivers/gpu/drm/drm_mipi_dbi.c  |   2 +-
 drivers/gpu/drm/gud/gud_pipe.c  |  10 +--
 drivers/gpu/drm/tiny/cirrus.c   |  10 +--
 drivers/gpu/drm/tiny/repaper.c  |   2 +-
 drivers/gpu/drm/tiny/st7586.c   |   2 +-
 include/drm/drm_format_helper.h |  30 +++
 7 files changed, 97 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c 
b/drivers/gpu/drm/drm_format_helper.c
index 79869ed553d9..260dc587c1d7 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -156,7 +156,7 @@ void drm_fb_swab(void *dst, unsigned int dst_pitch, const 
void *src,
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
-static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned 
int pixels)
+static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, const __le32 *sbuf, 
unsigned int pixels)
 {
unsigned int x;
u32 pix;
@@ -172,23 +172,24 @@ static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, 
__le32 *sbuf, unsigned int
 /**
  * drm_fb_xrgb_to_rgb332 - Convert XRGB to RGB332 clip buffer
  * @dst: RGB332 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: XRGB source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * Drivers can use this function for RGB332 devices that don't natively 
support XRGB.
- *
- * This function does not apply clipping on dst, i.e. the destination is a 
small buffer
- * containing the clip rect only.
  */
-void drm_fb_xrgb_to_rgb332(void *dst, void *src, struct drm_framebuffer 
*fb,
-  struct drm_rect *clip)
+void drm_fb_xrgb_to_rgb332(void *dst, unsigned int dst_pitch, const void 
*src,
+  const struct drm_framebuffer *fb, const struct 
drm_rect *clip)
 {
size_t width = drm_rect_width(clip);
size_t src_len = width * sizeof(u32);
unsigned int y;
void *sbuf;
 
+   if (!dst_pitch)
+   dst_pitch = width;
+
/* Use a buffer to speed up access on buffers with uncached read 
mapping (i.e. WC) */
sbuf = kmalloc(src_len, GFP_KERNEL);
if (!sbuf)
@@ -199,14 +200,14 @@ void drm_fb_xrgb_to_rgb332(void *dst, void *src, 
struct drm_framebuffer *fb,
memcpy(sbuf, src, src_len);
drm_fb_xrgb_to_rgb332_line(dst, sbuf, width);
src += fb->pitches[0];
-   dst += width;
+   dst += dst_pitch;
}
 
kfree(sbuf);
 }
 EXPORT_SYMBOL(drm_fb_xrgb_to_rgb332);
 
-static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 *sbuf,
+static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, const u32 *sbuf,
   unsigned int pixels,
   bool swab)
 {
@@ -227,6 +228,7 @@ static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 
*sbuf,
 /**
  * drm_fb_xrgb_to_rgb565 - Convert XRGB to RGB565 clip buffer
  * @dst: RGB565 destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: XRGB source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -234,13 +236,10 @@ static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 
*sbuf,
  *
  * Drivers can use this function for RGB565 devices that don't natively
  * support XRGB.
- *
- * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
  */
-void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr,
-  struct drm_framebuffer *fb,
-  struct drm_rect *clip, bool swab)
+void drm_fb_xrgb_to_rgb565(void *dst, unsigned int dst_pitch, const void 
*vaddr,
+  const struct drm_framebuffer *fb, const struct 
drm_rect *clip,
+  bool swab)
 {
size_t linepixels = clip->x2 - clip->x1;
size_t src_len = linepixels * sizeof(u32);
@@ -248,6 +247,9 @@ void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr,
unsigned y, lines = clip->y2 - clip->y1;
void *sbuf;
 
+   if (!dst_pitch)
+   dst_pitch = dst_len;
+
/*
 * The cma memory is write-combined so reads are uncached.
 * Speed up by fetching one line at a time.
@@ -261,7 +263,7 @@ void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr,

[PATCH 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()

2021-10-22 Thread Thomas Zimmermann
Add destination-buffer pitch as argument to drm_fb_swab(). Done for
consistency with the rest of the interface.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_format_helper.c | 19 +++
 drivers/gpu/drm/drm_mipi_dbi.c  |  2 +-
 drivers/gpu/drm/gud/gud_pipe.c  |  2 +-
 include/drm/drm_format_helper.h |  5 +++--
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c 
b/drivers/gpu/drm/drm_format_helper.c
index 38c8055f6fa8..79869ed553d9 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
 /**
  * drm_fb_swab - Swap bytes into clip buffer
  * @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @src: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
@@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio);
  * This function does not apply clipping on dst, i.e. the destination
  * is a small buffer containing the clip rect only.
  */
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-struct drm_rect *clip, bool cached)
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+const struct drm_framebuffer *fb, const struct drm_rect *clip,
+bool cached)
 {
u8 cpp = fb->format->cpp[0];
size_t len = drm_rect_width(clip) * cpp;
-   u16 *src16, *dst16 = dst;
-   u32 *src32, *dst32 = dst;
+   const u16 *src16;
+   const u32 *src32;
+   u16 *dst16;
+   u32 *dst32;
unsigned int x, y;
void *buf = NULL;
 
if (WARN_ON_ONCE(cpp != 2 && cpp != 4))
return;
 
+   if (!dst_pitch)
+   dst_pitch = len;
+
if (!cached)
buf = kmalloc(len, GFP_KERNEL);
 
@@ -131,6 +138,9 @@ void drm_fb_swab(void *dst, void *src, struct 
drm_framebuffer *fb,
src32 = src;
}
 
+   dst16 = dst;
+   dst32 = dst;
+
for (x = clip->x1; x < clip->x2; x++) {
if (cpp == 4)
*dst32++ = swab32(*src32++);
@@ -139,6 +149,7 @@ void drm_fb_swab(void *dst, void *src, struct 
drm_framebuffer *fb,
}
 
src += fb->pitches[0];
+   dst += dst_pitch;
}
 
kfree(buf);
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index c09df8b06c7a..7ce89917d761 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -211,7 +211,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
switch (fb->format->format) {
case DRM_FORMAT_RGB565:
if (swap)
-   drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
+   drm_fb_swab(dst, 0, src, fb, clip, !gem->import_attach);
else
drm_fb_memcpy(dst, 0, src, fb, clip);
break;
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index a92112ffd37a..e0b117b2559f 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -201,7 +201,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct 
drm_framebuffer *fb,
len = gud_xrgb_to_color(buf, format, vaddr, fb, 
rect);
}
} else if (gud_is_big_endian() && format->cpp[0] > 1) {
-   drm_fb_swab(buf, vaddr, fb, rect, !import_attach);
+   drm_fb_swab(buf, 0, vaddr, fb, rect, !import_attach);
} else if (compression && !import_attach && pitch == fb->pitches[0]) {
/* can compress directly from the framebuffer */
buf = vaddr + rect->y1 * pitch;
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 8d72f6fd27e9..c4c3c845e119 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -17,8 +17,9 @@ void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const 
void *vaddr,
   const struct drm_framebuffer *fb, const struct drm_rect 
*clip);
 void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void 
*vaddr,
const struct drm_framebuffer *fb, const struct drm_rect 
*clip);
-void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
-struct drm_rect *clip, bool cached);
+void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src,
+const struct drm_framebuffer *fb, const struct drm_rect *clip,
+bool cached);
 void drm_fb_xrgb_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer 
*fb,
   struct drm_rect *clip);
 void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr,
-- 
2.33.0


[PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()

2021-10-22 Thread Thomas Zimmermann
Provide a function that computes the offset into a blit destination
buffer. This will allow to move destination-buffer clipping into the
format-helper callers.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_format_helper.c | 10 --
 include/drm/drm_format_helper.h |  4 
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c 
b/drivers/gpu/drm/drm_format_helper.c
index 69fde60e36b3..28e9d0d89270 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -17,12 +17,18 @@
 #include 
 #include 
 
-static unsigned int clip_offset(struct drm_rect *clip,
-   unsigned int pitch, unsigned int cpp)
+static unsigned int clip_offset(const struct drm_rect *clip, unsigned int 
pitch, unsigned int cpp)
 {
return clip->y1 * pitch + clip->x1 * cpp;
 }
 
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct 
drm_format_info *format,
+const struct drm_rect *clip)
+{
+   return clip_offset(clip, pitch, format->cpp[0]);
+}
+EXPORT_SYMBOL(drm_fb_clip_offset);
+
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index e86925cf07b9..90b9bd9ecb83 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -6,9 +6,13 @@
 #ifndef __LINUX_DRM_FORMAT_HELPER_H
 #define __LINUX_DRM_FORMAT_HELPER_H
 
+struct drm_format_info;
 struct drm_framebuffer;
 struct drm_rect;
 
+unsigned long drm_fb_clip_offset(unsigned int pitch, const struct 
drm_format_info *format,
+const struct drm_rect *clip);
+
 void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
   struct drm_rect *clip);
 void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void 
*vaddr,
-- 
2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 2/9] drm/format-helper: Rework format-helper memcpy functions

2021-10-22 Thread Thomas Zimmermann
Move destination-buffer clipping from all format-helper memcpy
function into callers. Support destination-buffer pitch. Only
distinguish between system and I/O memory, but use same logic
everywhere.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_format_helper.c | 35 -
 drivers/gpu/drm/drm_mipi_dbi.c  |  2 +-
 drivers/gpu/drm/gud/gud_pipe.c  |  2 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  5 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c  |  4 ++-
 drivers/gpu/drm/tiny/cirrus.c   | 14 +
 include/drm/drm_format_helper.h |  9 +++---
 7 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c 
b/drivers/gpu/drm/drm_format_helper.c
index 28e9d0d89270..38c8055f6fa8 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -32,58 +32,62 @@ EXPORT_SYMBOL(drm_fb_clip_offset);
 /**
  * drm_fb_memcpy - Copy clip buffer
  * @dst: Destination buffer
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
  * This function does not apply clipping on dst, i.e. the destination
- * is a small buffer containing the clip rect only.
+ * is at the top-left corner.
  */
-void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
-  struct drm_rect *clip)
+void drm_fb_memcpy(void *dst, unsigned int dst_pitch, const void *vaddr,
+  const struct drm_framebuffer *fb, const struct drm_rect 
*clip)
 {
unsigned int cpp = fb->format->cpp[0];
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y, lines = clip->y2 - clip->y1;
 
+   if (!dst_pitch)
+   dst_pitch = len;
+
vaddr += clip_offset(clip, fb->pitches[0], cpp);
for (y = 0; y < lines; y++) {
memcpy(dst, vaddr, len);
vaddr += fb->pitches[0];
-   dst += len;
+   dst += dst_pitch;
}
 }
 EXPORT_SYMBOL(drm_fb_memcpy);
 
 /**
- * drm_fb_memcpy_dstclip - Copy clip buffer
+ * drm_fb_memcpy_toio - Copy clip buffer
  * @dst: Destination buffer (iomem)
  * @dst_pitch: Number of bytes between two consecutive scanlines within dst
  * @vaddr: Source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  *
- * This function applies clipping on dst, i.e. the destination is a
- * full (iomem) framebuffer but only the clip rect content is copied over.
+ * This function does not apply clipping on dst, i.e. the destination
+ * is at the top-left corner.
  */
-void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch,
-  void *vaddr, struct drm_framebuffer *fb,
-  struct drm_rect *clip)
+void drm_fb_memcpy_toio(void __iomem *dst, unsigned int dst_pitch, const void 
*vaddr,
+   const struct drm_framebuffer *fb, const struct drm_rect 
*clip)
 {
unsigned int cpp = fb->format->cpp[0];
-   unsigned int offset = clip_offset(clip, dst_pitch, cpp);
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y, lines = clip->y2 - clip->y1;
 
-   vaddr += offset;
-   dst += offset;
+   if (!dst_pitch)
+   dst_pitch = len;
+
+   vaddr += clip_offset(clip, fb->pitches[0], cpp);
for (y = 0; y < lines; y++) {
memcpy_toio(dst, vaddr, len);
vaddr += fb->pitches[0];
dst += dst_pitch;
}
 }
-EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
+EXPORT_SYMBOL(drm_fb_memcpy_toio);
 
 /**
  * drm_fb_swab - Swap bytes into clip buffer
@@ -472,7 +476,8 @@ int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned 
int dst_pitch,
dst_format = DRM_FORMAT_XRGB;
 
if (dst_format == fb_format) {
-   drm_fb_memcpy_dstclip(dst, dst_pitch, vmap, fb, clip);
+   dst += clip_offset(clip, dst_pitch, fb->format->cpp[0]);
+   drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip);
return 0;
 
} else if (dst_format == DRM_FORMAT_RGB565) {
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 71b646c4131f..c09df8b06c7a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -213,7 +213,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
if (swap)
drm_fb_swab(dst, src, fb, clip, !gem->import_attach);
else
-   drm_fb_memcpy(dst, src, fb, clip);
+   drm_fb_memcpy(dst, 0, src, fb, clip);
break;
case DRM_FORMAT_XRGB:
drm_fb_xrgb_to_rgb565(dst, src, fb, clip, swap);
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index daf75c178c2b..a92112ffd37a 100644
--- 

Re: [PATCH 0/5] iommu/virtio: Add identity domains

2021-10-22 Thread Jean-Philippe Brucker
On Fri, Oct 22, 2021 at 06:16:27AM -0400, Michael S. Tsirkin wrote:
> On Wed, Oct 13, 2021 at 01:10:48PM +0100, Jean-Philippe Brucker wrote:
> > Support identity domains, allowing to only enable IOMMU protection for a
> > subset of endpoints (those assigned to userspace, for example). Users
> > may enable identity domains at compile time
> > (CONFIG_IOMMU_DEFAULT_PASSTHROUGH), boot time (iommu.passthrough=1) or
> > runtime (/sys/kernel/iommu_groups/*/type = identity).
> 
> 
> I put this in my branch so it can get testing under linux-next,
> but pls notice if the ballot does not conclude in time
> for the merge window I won't send it to Linus.

Makes sense, thank you. I sent a new version of the spec change with
clarifications
https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07969.html

Thanks,
Jean

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-22 Thread Michael S. Tsirkin
On Fri, Oct 22, 2021 at 12:19:11AM -0500, Mike Christie wrote:
> This patch allows userspace to create workers and bind them to vqs. You
> can have N workers per dev and also share N workers with M vqs.
> 
> Signed-off-by: Mike Christie 
> ---
>  drivers/vhost/vhost.c| 99 
>  drivers/vhost/vhost.h|  2 +-
>  include/uapi/linux/vhost.h   | 11 
>  include/uapi/linux/vhost_types.h | 12 
>  4 files changed, 112 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 04f43a6445e1..c86e88d7f35c 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -493,7 +493,6 @@ void vhost_dev_init(struct vhost_dev *dev,
>   dev->umem = NULL;
>   dev->iotlb = NULL;
>   dev->mm = NULL;
> - dev->worker = NULL;
>   dev->iov_limit = iov_limit;
>   dev->weight = weight;
>   dev->byte_weight = byte_weight;
> @@ -576,20 +575,40 @@ static void vhost_worker_stop(struct vhost_worker 
> *worker)
>   wait_for_completion(worker->exit_done);
>  }
>  
> -static void vhost_worker_free(struct vhost_dev *dev)
> -{
> - struct vhost_worker *worker = dev->worker;
>  
> +static void vhost_worker_put(struct vhost_worker *worker)
> +{
>   if (!worker)
>   return;
>  
> - dev->worker = NULL;
> + if (!refcount_dec_and_test(>refcount))
> + return;
> +
>   WARN_ON(!llist_empty(>work_list));
>   vhost_worker_stop(worker);
>   kfree(worker);
>  }
>  
> -static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
> +static void vhost_vq_clear_worker(struct vhost_virtqueue *vq)
> +{
> + if (vq->worker)
> + vhost_worker_put(vq->worker);
> + vq->worker = NULL;
> +}
> +
> +static void vhost_workers_free(struct vhost_dev *dev)
> +{
> + int i;
> +
> + if (!dev->use_worker)
> + return;
> +
> + for (i = 0; i < dev->nvqs; i++)
> + vhost_vq_clear_worker(dev->vqs[i]);
> +}
> +
> +static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev,
> + int init_vq_map_count)
>  {
>   struct vhost_worker *worker;
>   struct task_struct *task;
> @@ -598,9 +617,9 @@ static struct vhost_worker *vhost_worker_create(struct 
> vhost_dev *dev)
>   if (!worker)
>   return NULL;
>  
> - dev->worker = worker;
>   worker->kcov_handle = kcov_common_handle();
>   init_llist_head(>work_list);
> + refcount_set(>refcount, init_vq_map_count);
>  
>   /*
>* vhost used to use the kthread API which ignores all signals by
> @@ -617,10 +636,58 @@ static struct vhost_worker *vhost_worker_create(struct 
> vhost_dev *dev)
>  
>  free_worker:
>   kfree(worker);
> - dev->worker = NULL;
>   return NULL;
>  }
>  
> +static struct vhost_worker *vhost_worker_find(struct vhost_dev *dev, pid_t 
> pid)
> +{
> + struct vhost_worker *worker = NULL;
> + int i;
> +
> + for (i = 0; i < dev->nvqs; i++) {
> + if (dev->vqs[i]->worker->task->pid != pid)
> + continue;
> +
> + worker = dev->vqs[i]->worker;
> + break;
> + }
> +
> + return worker;
> +}
> +
> +/* Caller must have device mutex */
> +static int vhost_vq_setup_worker(struct vhost_virtqueue *vq,
> +  struct vhost_vring_worker *info)
> +{
> + struct vhost_dev *dev = vq->dev;
> + struct vhost_worker *worker;
> +
> + if (!dev->use_worker)
> + return -EINVAL;
> +
> + /* We don't support setting a worker on an active vq */
> + if (vq->private_data)
> + return -EBUSY;
> +
> + if (info->pid == VHOST_VRING_NEW_WORKER) {
> + worker = vhost_worker_create(dev, 1);
> + if (!worker)
> + return -ENOMEM;
> +
> + info->pid = worker->task->pid;
> + } else {
> + worker = vhost_worker_find(dev, info->pid);
> + if (!worker)
> + return -ENODEV;
> +
> + refcount_inc(>refcount);
> + }
> +
> + vhost_vq_clear_worker(vq);
> + vq->worker = worker;
> + return 0;
> +}
> +
>  /* Caller should have device mutex */
>  long vhost_dev_set_owner(struct vhost_dev *dev)
>  {
> @@ -636,7 +703,7 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
>   vhost_attach_mm(dev);
>  
>   if (dev->use_worker) {
> - worker = vhost_worker_create(dev);
> + worker = vhost_worker_create(dev, dev->nvqs);
>   if (!worker)
>   goto err_worker;
>  
> @@ -650,7 +717,7 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
>  
>   return 0;
>  err_iovecs:
> - vhost_worker_free(dev);
> + vhost_workers_free(dev);
>  err_worker:
>   vhost_detach_mm(dev);
>  err_mm:
> @@ -742,7 +809,7 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
>   dev->iotlb = NULL;
>   

Re: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

2021-10-22 Thread Michael S. Tsirkin
On Thu, Oct 21, 2021 at 07:35:01PM +0300, Parav Pandit wrote:
> Currently user cannot view the vdpa device config space. Also user
> cannot set the mac address and mtu of the vdpa device.
> This patchset enables users to set the mac address and mtu of the vdpa
> device once the device is created.
> If a vendor driver supports such configuration user can set it otherwise
> user gets unsupported error.
> 
> vdpa mac address and mtu are device configuration layout fields.
> To keep interface generic enough for multiple types of vdpa devices, mac
> address and mtu setting is implemented as configuration layout fields.


So I got lots of "sha1 is lacking or useless" warnings with this.
I did my best to merge but please check out the result in
linux-next.

> An example of query & set of config layout fields for vdpa_sim_net
> driver:
> 
> Configuration layout fields are set when a vdpa device is created.
> 
> $ vdpa mgmtdev show
> vdpasim_net:
>   supported_classes net
> pci/:08:00.2:
>   supported_classes net
> 
> Add the device with MAC and MTU:
> $ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:66 mtu 1500
> 
> In above command only mac address or only mtu can also be set.
> 
> View the config after setting:
> $ vdpa dev config show
> bar: mac 00:11:22:33:44:66 link up link_announce false mtu 1500
> 
> Patch summary:
> Patch-1 introduced and use helpers for get/set config area
> Patch-2 implement query device config layout
> Patch-3 use kernel coding style for structure comment
> Patch-4 enanble user to set mac and mtu in config space
> Patch-5 vdpa_sim_net implements get and set of config layout
> Patch-6 mlx vdpa driver fix to avoid clearing VIRTIO_NET_F_MAC during
> reset callback
> Patch-7 mlx5 vdpa driver supports user provided mac config
> Patch-8 mlx5 vdpa driver uses user provided mac during rx flow steering
> 
> changelog:
> v3->v4:
>  - enable setting mac and mtu of the vdpa device using creation time
>  - introduced a patch to fix mlx5 driver to avoid clearing
>VIRTIO_NET_F_MAC
>  - introduced a patch to use kernel coding style for structure comment
>  - removed config attributes not used by sim and mlx5 net drivers
> v2->v3:
>  - dropped patches which are merged
>  - simplified code to handle non transitional devices
> 
> v1->v2:
>  - new patches to fix kdoc comment to add new kdoc section
>  - new patch to have synchronized access to features and config space
>  - read whole net config layout instead of individual fields
>  - added error extack for unmanaged vdpa device
>  - fixed several endianness issues
>  - introduced vdpa device ops for get config which is synchronized
>with other get/set features ops and config ops
>  - fixed mtu range checking for max
>  - using NLA_POLICY_ETH_ADDR
>  - set config moved to device ops instead of mgmtdev ops
>  - merged build and set to single routine
>  - ensuring that user has NET_ADMIN capability for configuring network
>attributes
>  - using updated interface and callbacks for get/set config
>  - following new api for config get/set for mgmt tool in mlx5 vdpa
>driver
>  - fixes for accessing right SF dma device and bar address
>  - fix for mtu calculation
>  - fix for bit access in features
>  - fix for index restore with suspend/resume operation
> 
> 
> Eli Cohen (2):
>   vdpa/mlx5: Support configuration of MAC
>   vdpa/mlx5: Forward only packets with allowed MAC address
> 
> Parav Pandit (6):
>   vdpa: Introduce and use vdpa device get, set config helpers
>   vdpa: Introduce query of device config layout
>   vdpa: Use kernel coding style for structure comments
>   vdpa: Enable user to set mac and mtu of vdpa device
>   vdpa_sim_net: Enable user to set mac address and mtu
>   vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit
> 
>  drivers/vdpa/ifcvf/ifcvf_main.c  |   3 +-
>  drivers/vdpa/mlx5/net/mlx5_vnet.c|  92 +++---
>  drivers/vdpa/vdpa.c  | 243 ++-
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |   3 +-
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  38 +++--
>  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
>  drivers/vhost/vdpa.c |   3 +-
>  include/linux/vdpa.h |  47 --
>  include/uapi/linux/vdpa.h|   6 +
>  9 files changed, 375 insertions(+), 63 deletions(-)
> 
> -- 
> 2.25.4

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/5] iommu/virtio: Add identity domains

2021-10-22 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 01:10:48PM +0100, Jean-Philippe Brucker wrote:
> Support identity domains, allowing to only enable IOMMU protection for a
> subset of endpoints (those assigned to userspace, for example). Users
> may enable identity domains at compile time
> (CONFIG_IOMMU_DEFAULT_PASSTHROUGH), boot time (iommu.passthrough=1) or
> runtime (/sys/kernel/iommu_groups/*/type = identity).


I put this in my branch so it can get testing under linux-next,
but pls notice if the ballot does not conclude in time
for the merge window I won't send it to Linus.

> Patches 1-2 support identity domains using the optional
> VIRTIO_IOMMU_F_BYPASS_CONFIG feature. The feature bit is not yet in the
> spec, see [1] for the latest proposal.
> 
> Patches 3-5 add a fallback to identity mappings, when the feature is not
> supported.
> 
> Note that this series doesn't touch the global bypass bit added by
> VIRTIO_IOMMU_F_BYPASS_CONFIG. All endpoints managed by the IOMMU should
> be attached to a domain, so global bypass isn't in use after endpoints
> are probed. Before that, the global bypass policy is decided by the
> hypervisor and firmware. So I don't think Linux needs to touch the
> global bypass bit, but there are some patches available on my
> virtio-iommu/bypass branch [2] to test it.
> 
> QEMU patches are on my virtio-iommu/bypass branch [3] (and the list)
> 
> [1] https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07898.html
> [2] https://jpbrucker.net/git/linux/log/?h=virtio-iommu/bypass
> [3] https://jpbrucker.net/git/qemu/log/?h=virtio-iommu/bypass
> 
> Jean-Philippe Brucker (5):
>   iommu/virtio: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG
>   iommu/virtio: Support bypass domains
>   iommu/virtio: Sort reserved regions
>   iommu/virtio: Pass end address to viommu_add_mapping()
>   iommu/virtio: Support identity-mapped domains
> 
>  include/uapi/linux/virtio_iommu.h |   8 ++-
>  drivers/iommu/virtio-iommu.c  | 113 +-
>  2 files changed, 101 insertions(+), 20 deletions(-)
> 
> -- 
> 2.33.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V7] gpio: virtio: Add IRQ support

2021-10-22 Thread Michael S. Tsirkin
On Thu, Oct 21, 2021 at 04:34:19PM +0530, Viresh Kumar wrote:
> This patch adds IRQ support for the virtio GPIO driver. Note that this
> uses the irq_bus_lock/unlock() callbacks, since those operations over
> virtio may sleep.
> 
> Reviewed-by: Linus Walleij 
> Signed-off-by: Viresh Kumar 

Acked-by: Michael S. Tsirkin 

I think this can be merged - while ballot did not close yet
you already have a majority vote Yes. Worst case we'll revert
but I don't expect that.


> ---
> Bartosz,
> 
> The spec changes are close to merging now, I will let you know once the ballot
> is closed and the spec changes are merged. You can then pick this patch for
> 5.16.
> 
> V6->V7:
> - Use generic_handle_domain_irq.
> - Drop check for IRQ_TYPE_NONE, dead code.
> - Avoid breaking line to fit into 80 columns.
> 
> V5->V6:
> - Sent it separately as the other patches are already merged.
> - Queue the buffers only after enabling the irq (as per latest spec).
> - Migrate to raw_spinlock_t.
> 
>  drivers/gpio/Kconfig |   1 +
>  drivers/gpio/gpio-virtio.c   | 302 ++-
>  include/uapi/linux/virtio_gpio.h |  25 +++
>  3 files changed, 324 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index fae5141251e5..bfa723ff8e7c 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1674,6 +1674,7 @@ config GPIO_MOCKUP
>  config GPIO_VIRTIO
>   tristate "VirtIO GPIO support"
>   depends on VIRTIO
> + select GPIOLIB_IRQCHIP
>   help
> Say Y here to enable guest support for virtio-based GPIO controllers.
>  
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> index d24f1c9264bc..aeec4bf0b625 100644
> --- a/drivers/gpio/gpio-virtio.c
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -28,12 +29,30 @@ struct virtio_gpio_line {
>   unsigned int rxlen;
>  };
>  
> +struct vgpio_irq_line {
> + u8 type;
> + bool disabled;
> + bool masked;
> + bool queued;
> + bool update_pending;
> + bool queue_pending;
> +
> + struct virtio_gpio_irq_request ireq cacheline_aligned;
> + struct virtio_gpio_irq_response ires cacheline_aligned;
> +};
> +
>  struct virtio_gpio {
>   struct virtio_device *vdev;
>   struct mutex lock; /* Protects virtqueue operation */
>   struct gpio_chip gc;
>   struct virtio_gpio_line *lines;
>   struct virtqueue *request_vq;
> +
> + /* irq support */
> + struct virtqueue *event_vq;
> + struct mutex irq_lock; /* Protects irq operation */
> + raw_spinlock_t eventq_lock; /* Protects queuing of the buffer */
> + struct vgpio_irq_line *irq_lines;
>  };
>  
>  static int _virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio,
> @@ -186,6 +205,238 @@ static void virtio_gpio_set(struct gpio_chip *gc, 
> unsigned int gpio, int value)
>   virtio_gpio_req(vgpio, VIRTIO_GPIO_MSG_SET_VALUE, gpio, value, NULL);
>  }
>  
> +/* Interrupt handling */
> +static void virtio_gpio_irq_prepare(struct virtio_gpio *vgpio, u16 gpio)
> +{
> + struct vgpio_irq_line *irq_line = >irq_lines[gpio];
> + struct virtio_gpio_irq_request *ireq = _line->ireq;
> + struct virtio_gpio_irq_response *ires = _line->ires;
> + struct scatterlist *sgs[2], req_sg, res_sg;
> + int ret;
> +
> + if (WARN_ON(irq_line->queued || irq_line->masked || irq_line->disabled))
> + return;
> +
> + ireq->gpio = cpu_to_le16(gpio);
> + sg_init_one(_sg, ireq, sizeof(*ireq));
> + sg_init_one(_sg, ires, sizeof(*ires));
> + sgs[0] = _sg;
> + sgs[1] = _sg;
> +
> + ret = virtqueue_add_sgs(vgpio->event_vq, sgs, 1, 1, irq_line, 
> GFP_ATOMIC);
> + if (ret) {
> + dev_err(>vdev->dev, "failed to add request to eventq\n");
> + return;
> + }
> +
> + irq_line->queued = true;
> + virtqueue_kick(vgpio->event_vq);
> +}
> +
> +static void virtio_gpio_irq_enable(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct virtio_gpio *vgpio = gpiochip_get_data(gc);
> + struct vgpio_irq_line *irq_line = >irq_lines[d->hwirq];
> +
> + raw_spin_lock(>eventq_lock);
> + irq_line->disabled = false;
> + irq_line->masked = false;
> + irq_line->queue_pending = true;
> + raw_spin_unlock(>eventq_lock);
> +
> + irq_line->update_pending = true;
> +}
> +
> +static void virtio_gpio_irq_disable(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct virtio_gpio *vgpio = gpiochip_get_data(gc);
> + struct vgpio_irq_line *irq_line = >irq_lines[d->hwirq];
> +
> + raw_spin_lock(>eventq_lock);
> + irq_line->disabled = true;
> + irq_line->masked = true;
> + irq_line->queue_pending = false;
> + raw_spin_unlock(>eventq_lock);
> +
> + irq_line->update_pending = true;
> 

Re: [PATCH V4 2/8] fork: move PF_IO_WORKER's kernel frame setup to new flag

2021-10-22 Thread Michael S. Tsirkin
On Thu, Oct 07, 2021 at 04:44:42PM -0500, Mike Christie wrote:
> The vhost worker threads need the same frame setup as io_uring's worker
> threads, but handle signals differently and do not need the same
> scheduling behavior. This patch separate's the frame setup parts of
> PF_IO_WORKER into a new PF flag PF_USER_WORKER.
> 
> Signed-off-by: Mike Christie 
> ---
>  arch/alpha/kernel/process.c  | 2 +-
>  arch/arc/kernel/process.c| 2 +-
>  arch/arm/kernel/process.c| 2 +-
>  arch/arm64/kernel/process.c  | 2 +-
>  arch/csky/kernel/process.c   | 2 +-
>  arch/h8300/kernel/process.c  | 2 +-
>  arch/hexagon/kernel/process.c| 2 +-
>  arch/ia64/kernel/process.c   | 2 +-
>  arch/m68k/kernel/process.c   | 2 +-
>  arch/microblaze/kernel/process.c | 2 +-
>  arch/mips/kernel/process.c   | 2 +-
>  arch/nds32/kernel/process.c  | 2 +-
>  arch/nios2/kernel/process.c  | 2 +-
>  arch/openrisc/kernel/process.c   | 2 +-
>  arch/parisc/kernel/process.c | 2 +-
>  arch/powerpc/kernel/process.c| 2 +-
>  arch/riscv/kernel/process.c  | 2 +-
>  arch/s390/kernel/process.c   | 2 +-
>  arch/sh/kernel/process_32.c  | 2 +-
>  arch/sparc/kernel/process_32.c   | 2 +-
>  arch/sparc/kernel/process_64.c   | 2 +-
>  arch/um/kernel/process.c | 2 +-
>  arch/x86/kernel/process.c| 2 +-
>  arch/xtensa/kernel/process.c | 2 +-
>  include/linux/sched.h| 1 +
>  include/linux/sched/task.h   | 1 +
>  kernel/fork.c| 4 +++-
>  27 files changed, 29 insertions(+), 25 deletions(-)


For something that's touching include/linux/sched.h
and all arches at once, this has not been CC'd widely enough.


> diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
> index a5123ea426ce..e350fff2ea14 100644
> --- a/arch/alpha/kernel/process.c
> +++ b/arch/alpha/kernel/process.c
> @@ -249,7 +249,7 @@ int copy_thread(unsigned long clone_flags, unsigned long 
> usp,
>   childti->pcb.ksp = (unsigned long) childstack;
>   childti->pcb.flags = 1; /* set FEN, clear everything else */
>  
> - if (unlikely(p->flags & (PF_KTHREAD | PF_IO_WORKER))) {
> + if (unlikely(p->flags & (PF_KTHREAD | PF_USER_WORKER))) {
>   /* kernel thread */
>   memset(childstack, 0,
>   sizeof(struct switch_stack) + sizeof(struct pt_regs));
> diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c
> index 3793876f42d9..c3f4952cce17 100644
> --- a/arch/arc/kernel/process.c
> +++ b/arch/arc/kernel/process.c
> @@ -191,7 +191,7 @@ int copy_thread(unsigned long clone_flags, unsigned long 
> usp,
>   childksp[0] = 0;/* fp */
>   childksp[1] = (unsigned long)ret_from_fork; /* blink */
>  
> - if (unlikely(p->flags & (PF_KTHREAD | PF_IO_WORKER))) {
> + if (unlikely(p->flags & (PF_KTHREAD | PF_USER_WORKER))) {
>   memset(c_regs, 0, sizeof(struct pt_regs));
>  
>   c_callee->r13 = kthread_arg;
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 0e2d3051741e..449c9db3942a 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -247,7 +247,7 @@ int copy_thread(unsigned long clone_flags, unsigned long 
> stack_start,
>   thread->cpu_domain = get_domain();
>  #endif
>  
> - if (likely(!(p->flags & (PF_KTHREAD | PF_IO_WORKER {
> + if (likely(!(p->flags & (PF_KTHREAD | PF_USER_WORKER {
>   *childregs = *current_pt_regs();
>   childregs->ARM_r0 = 0;
>   if (stack_start)
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 40adb8cdbf5a..e2fe88a3ae90 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -333,7 +333,7 @@ int copy_thread(unsigned long clone_flags, unsigned long 
> stack_start,
>  
>   ptrauth_thread_init_kernel(p);
>  
> - if (likely(!(p->flags & (PF_KTHREAD | PF_IO_WORKER {
> + if (likely(!(p->flags & (PF_KTHREAD | PF_USER_WORKER {
>   *childregs = *current_pt_regs();
>   childregs->regs[0] = 0;
>  
> diff --git a/arch/csky/kernel/process.c b/arch/csky/kernel/process.c
> index 3d0ca22cd0e2..509f2bfe4ace 100644
> --- a/arch/csky/kernel/process.c
> +++ b/arch/csky/kernel/process.c
> @@ -49,7 +49,7 @@ int copy_thread(unsigned long clone_flags,
>   /* setup thread.sp for switch_to !!! */
>   p->thread.sp = (unsigned long)childstack;
>  
> - if (unlikely(p->flags & (PF_KTHREAD | PF_IO_WORKER))) {
> + if (unlikely(p->flags & (PF_KTHREAD | PF_USER_WORKER))) {
>   memset(childregs, 0, sizeof(struct pt_regs));
>   childstack->r15 = (unsigned long) ret_from_kernel_thread;
>   childstack->r10 = kthread_arg;
> diff --git a/arch/h8300/kernel/process.c b/arch/h8300/kernel/process.c
> index 2ac27e4248a4..11baf058b6c5 100644
> --- a/arch/h8300/kernel/process.c
> +++ b/arch/h8300/kernel/process.c

Re: [PATCH V3 00/11] vhost: multiple worker support

2021-10-22 Thread Michael S. Tsirkin
On Fri, Oct 22, 2021 at 01:02:19AM -0500, michael.chris...@oracle.com wrote:
> On 10/22/21 12:18 AM, Mike Christie wrote:
> > Results:
> > 
> > 
> > fio jobs1   2   4   8   12  16
> > --
> > 1 worker84k492k510k-   -   -
> 
> That should be 184k above.

Nice. I'd like to merge this but blocked because of a dependency
(since we can't allow userspace to create threads without any limit).

> > worker per vq   184k   380k744k1422k   2256k   2434k

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V3 00/11] vhost: multiple worker support

2021-10-22 Thread Michael S. Tsirkin
On Fri, Oct 22, 2021 at 12:18:59AM -0500, Mike Christie wrote:
> The following patches apply over linus's tree and this patchset
> 
> https://lore.kernel.org/all/20211007214448.6282-1-michael.chris...@oracle.com/
> 
> which allows us to check the vhost owner thread's RLIMITs:


Unfortunately that patchset in turn triggers kbuild warnings.
I was hoping you would address them, I don't think
merging that patchset before kbuild issues are addressed
is possible.

It also doesn't have lots of acks, I'm a bit apprehensive
of merging core changes like this through the vhost tree.
Try to CC more widely/ping people?

> It looks like that patchset has been ok'd by all the major parties
> and just needs a small cleanup to apply to Jens and Paul trees, so I
> wanted to post my threading patches based over it for review.
> 
> The following patches allow us to support multiple vhost workers per
> device. I ended up just doing Stefan's original idea where userspace has
> the kernel create a worker and we pass back the pid. This has the benefit
> over the workqueue and userspace thread approach where we only have
> one'ish code path in the kernel during setup to detect old tools. The
> main IO paths and device/vq setup/teardown paths all use common code.
> 
> I've also included a patch for qemu so you can get an idea of how it
> works. If we are ok with the kernel code then I'll break that up into
> a patchset and send to qemu-devel for review.
> 
> Results:
> 
> 
> fio jobs1   2   4   8   12  16
> --
> 1 worker84k492k510k-   -   -
> worker per vq   184k   380k744k1422k   2256k   2434k
> 
> Notes:
> 0. This used a simple fio command:
> 
> fio --filename=/dev/sdb  --direct=1 --rw=randrw --bs=4k \
> --ioengine=libaio --iodepth=128  --numjobs=$JOBS_ABOVE
> 
> and I used a VM with 16 vCPUs and 16 virtqueues.
> 
> 1. The patches were tested with emulate_pr=0 and these patches:
> 
> https://lore.kernel.org/all/yq1tuhge4bg@ca-mkp.ca.oracle.com/t/
> 
> which are in mkp's scsi branches for the next kernel. They fix the perf
> issues where IOPs dropped at 12 vqs/jobs.
> 
> 2. Because we have a hard limit of 1024 cmds, if the num jobs * iodepth
> was greater than 1024, I would decrease iodepth. So 12 jobs used 85 cmds,
> and 16 used 64.
> 
> 3. The perf issue above at 2 jobs is because when we only have 1 worker
> we execute more cmds per vhost_work due to all vqs funneling to one worker.
> This results in less context switches and better batching without having to
> tweak any settings. I'm working on patches to add back batching during lio
> completion and do polling on the submission side.
> 
> We will still want the threading patches, because if we batch at the fio
> level plus use the vhost theading patches, we can see a big boost like
> below. So hopefully doing it at the kernel will allow apps to just work
> without having to be smart like fio.
> 
> fio using io_uring and batching with the iodepth_batch* settings:
> 
> fio jobs1   2   4   8   12  16
> -
> 1 worker494k520k-   -   -   -
> worker per vq   496k878k1542k   2436k   2304k   2590k
> 
> V3:
> - fully convert vhost code to use vq based APIs instead of leaving it
> half per dev and half per vq.
> - rebase against kernel worker API.
> - Drop delayed worker creation. We always create the default worker at
> VHOST_SET_OWNER time. Userspace can create and bind workers after that.
> 
> v2:
> - change loop that we take a refcount to the worker in
> - replaced pid == -1 with define.
> - fixed tabbing/spacing coding style issue
> - use hash instead of list to lookup workers.
> - I dropped the patch that added an ioctl cmd to get a vq's worker's
> pid. I saw we might do a generic netlink interface instead.
> 
> 
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 6/6] vp_vdpa: introduce legacy virtio pci driver

2021-10-22 Thread Michael S. Tsirkin
On Fri, Sep 10, 2021 at 05:57:20AM -0400, Michael S. Tsirkin wrote:
> On Fri, Sep 10, 2021 at 10:28:18AM +0800, Wu Zongyong wrote:
> > On Thu, Sep 09, 2021 at 09:57:50AM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Sep 08, 2021 at 08:20:37PM +0800, Wu Zongyong wrote:
> > > > This patch implements a vdpa driver for legacy virtio-pci device. And
> > > > this has been tested with the ENI(Elastic Network Interface) which is a
> > > > hardware virtio network device in Alibaba ECS baremetal instance.
> > > > 
> > > > Note that legacy device doesn't support to change the virtqueue size, so
> > > > users should use get_vq_num_unchangeable callback to check if the
> > > > virqueue size can be changed.
> > > 
> > > Hmm isn't this implicit in this being a legacy device?
> > > 
> > Yes, but there is no way to report the backend device is legacy for
> > users. So I add a new callback get_vq_num_unchangeable to indicate it.
> 
> Legacy is actually easy. It does not have VIRTIO_F_VERSION_1.
> Transitional devices with two interfaces are trickier.
> 
> These really need an ioctl so userspace can tell vdpa
> whether it's being used through a legacy or modern interface.


Recently I proposed that a SET_FEATURES ioctl is used by QEMU when
guest accesses the device: through modern with VIRTIO_F_VERSION_1,
through legacy without.

What do you think?

> 
> > > > If not, users should get the virqueue size
> > > > first by the get_vq_num_max callback first then allocate same size
> > > > memory for the virtqueue otherwise the device won't work correctly.
> > > > 
> > > > Signed-off-by: Wu Zongyong 
> > > > ---
> > > >  drivers/vdpa/Kconfig |   7 +
> > > >  drivers/vdpa/virtio_pci/Makefile |   1 +
> > > >  drivers/vdpa/virtio_pci/vp_vdpa_common.c |   5 +
> > > >  drivers/vdpa/virtio_pci/vp_vdpa_common.h |  11 +
> > > >  drivers/vdpa/virtio_pci/vp_vdpa_legacy.c | 346 +++
> > > >  5 files changed, 370 insertions(+)
> > > >  create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa_legacy.c
> > > > 
> > > > diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
> > > > index a503c1b2bfd9..ccb4fdb11f0f 100644
> > > > --- a/drivers/vdpa/Kconfig
> > > > +++ b/drivers/vdpa/Kconfig
> > > > @@ -67,4 +67,11 @@ config VP_VDPA
> > > > help
> > > >   This kernel module bridges virtio PCI device to vDPA bus.
> > > >  
> > > > +config VP_VDPA_LEGACY
> > > > +   bool "Support legacy virtio pci device"
> > > > +   depends on VP_VDPA
> > > > +   select VIRTIO_PCI_LIB_LEGACY
> > > > +   help
> > > > + This option enables bridges legacy virito PCI device to vDPA 
> > > > bus.
> > > > +
> > > >  endif # VDPA
> > > > diff --git a/drivers/vdpa/virtio_pci/Makefile 
> > > > b/drivers/vdpa/virtio_pci/Makefile
> > > > index a772d86952b1..77c52dfb8b56 100644
> > > > --- a/drivers/vdpa/virtio_pci/Makefile
> > > > +++ b/drivers/vdpa/virtio_pci/Makefile
> > > > @@ -1,4 +1,5 @@
> > > >  # SPDX-License-Identifier: GPL-2.0
> > > >  
> > > >  vp_vdpa-y += vp_vdpa_common.o vp_vdpa_modern.o
> > > > +vp_vdpa-$(CONFIG_VP_VDPA_LEGACY) += vp_vdpa_legacy.o
> > > >  obj-$(CONFIG_VP_VDPA) += vp_vdpa.o
> > > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa_common.c 
> > > > b/drivers/vdpa/virtio_pci/vp_vdpa_common.c
> > > > index 3ff24c9ad6e4..fa91dc153244 100644
> > > > --- a/drivers/vdpa/virtio_pci/vp_vdpa_common.c
> > > > +++ b/drivers/vdpa/virtio_pci/vp_vdpa_common.c
> > > > @@ -8,6 +8,7 @@
> > > >   * Based on virtio_pci_modern.c.
> > > >   */
> > > >  
> > > > +#include "linux/err.h"
> > > >  #include 
> > > >  #include 
> > > >  #include "vp_vdpa_common.h"
> > > > @@ -172,6 +173,10 @@ static int vp_vdpa_probe(struct pci_dev *pdev, 
> > > > const struct pci_device_id *id)
> > > > return ret;
> > > >  
> > > > vp_vdpa = vp_vdpa_modern_probe(pdev);
> > > > +   if (PTR_ERR(vp_vdpa) == -ENODEV) {
> > > > +   dev_info(>dev, "Tring legacy driver");
> > > > +   vp_vdpa = vp_vdpa_legacy_probe(pdev);
> > > > +   }
> > > > if (IS_ERR(vp_vdpa))
> > > > return PTR_ERR(vp_vdpa);
> > > >  
> > > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa_common.h 
> > > > b/drivers/vdpa/virtio_pci/vp_vdpa_common.h
> > > > index 57886b55a2e9..39f241d8321b 100644
> > > > --- a/drivers/vdpa/virtio_pci/vp_vdpa_common.h
> > > > +++ b/drivers/vdpa/virtio_pci/vp_vdpa_common.h
> > > > @@ -10,6 +10,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  
> > > >  #define VP_VDPA_DRIVER_NAME "vp_vdpa"
> > > >  #define VP_VDPA_NAME_SIZE 256
> > > > @@ -26,6 +27,7 @@ struct vp_vdpa {
> > > > struct vdpa_device vdpa;
> > > > struct pci_dev *pci_dev;
> > > > struct virtio_pci_modern_device mdev;
> > > > +   struct virtio_pci_legacy_device ldev;
> > > > struct vp_vring *vring;
> > > > struct vdpa_callback config_cb;
> > > > char msix_name[VP_VDPA_NAME_SIZE];
> > > 

Re: [PATCH v3 1/1] virtio-blk: add num_request_queues module parameter

2021-10-22 Thread Michael S. Tsirkin
On Thu, Sep 02, 2021 at 11:46:22PM +0300, Max Gurtovoy wrote:
> Sometimes a user would like to control the amount of request queues to
> be created for a block device. For example, for limiting the memory
> footprint of virtio-blk devices.
> 
> Reviewed-by: Christoph Hellwig 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Max Gurtovoy 
> ---
> 
> changes from v2:
>  - renamed num_io_queues to num_request_queues (from Stefan)
>  - added Reviewed-by signatures (from Stefan and Christoph)
> 
> changes from v1:
>  - use param_set_uint_minmax (from Christoph)
>  - added "Should > 0" to module description
> 
> Note: This commit apply on top of Jens's branch for-5.15/drivers
> 
> ---
>  drivers/block/virtio_blk.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 4b49df2dfd23..aaa2833a4734 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -24,6 +24,23 @@
>  /* The maximum number of sg elements that fit into a virtqueue */
>  #define VIRTIO_BLK_MAX_SG_ELEMS 32768
>  
> +static int virtblk_queue_count_set(const char *val,
> + const struct kernel_param *kp)
> +{
> + return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
> +}
> +
> +static const struct kernel_param_ops queue_count_ops = {
> + .set = virtblk_queue_count_set,
> + .get = param_get_uint,
> +};
> +
> +static unsigned int num_request_queues;
> +module_param_cb(num_request_queues, _count_ops, _request_queues,
> + 0644);
> +MODULE_PARM_DESC(num_request_queues,
> +  "Number of request queues to use for blk device. Should > 0");
> +
>  static int major;
>  static DEFINE_IDA(vd_index_ida);
>  

I wasn't happy with the message here so I tweaked it.

Please look at it in linux-next and confirm. Thanks!


> @@ -501,7 +518,9 @@ static int init_vq(struct virtio_blk *vblk)
>   if (err)
>   num_vqs = 1;
>  
> - num_vqs = min_t(unsigned int, nr_cpu_ids, num_vqs);
> + num_vqs = min_t(unsigned int,
> + min_not_zero(num_request_queues, nr_cpu_ids),
> + num_vqs);
>  
>   vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
>   if (!vblk->vqs)
> -- 
> 2.18.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 1/1] virtio-blk: avoid preallocating big SGL for data

2021-10-22 Thread Michael S. Tsirkin
My tree is ok.
Looks like your patch was developed on top of some other tree,
not plan upstream linux, so git am fails. I applied it using
patch and some manual tweaking, and it seems to work for
me but please do test it in linux-next and confirm -
will push to a linux-next branch in my tree soon.

On Thu, Sep 23, 2021 at 04:40:56PM +0300, Max Gurtovoy wrote:
> Hi MST/Jens,
> 
> Do we need more review here or are we ok with the code and the test matrix ?
> 
> If we're ok, we need to decide if this goes through virtio PR or block PR.
> 
> Cheers,
> 
> -Max.
> 
> On 9/14/2021 3:22 PM, Stefan Hajnoczi wrote:
> > On Mon, Sep 13, 2021 at 05:50:21PM +0300, Max Gurtovoy wrote:
> > > On 9/6/2021 6:09 PM, Stefan Hajnoczi wrote:
> > > > On Wed, Sep 01, 2021 at 04:14:34PM +0300, Max Gurtovoy wrote:
> > > > > No need to pre-allocate a big buffer for the IO SGL anymore. If a 
> > > > > device
> > > > > has lots of deep queues, preallocation for the sg list can consume
> > > > > substantial amounts of memory. For HW virtio-blk device, nr_hw_queues
> > > > > can be 64 or 128 and each queue's depth might be 128. This means the
> > > > > resulting preallocation for the data SGLs is big.
> > > > > 
> > > > > Switch to runtime allocation for SGL for lists longer than 2 entries.
> > > > > This is the approach used by NVMe drivers so it should be reasonable 
> > > > > for
> > > > > virtio block as well. Runtime SGL allocation has always been the case
> > > > > for the legacy I/O path so this is nothing new.
> > > > > 
> > > > > The preallocated small SGL depends on SG_CHAIN so if the ARCH doesn't
> > > > > support SG_CHAIN, use only runtime allocation for the SGL.
> > > > > 
> > > > > Re-organize the setup of the IO request to fit the new sg chain
> > > > > mechanism.
> > > > > 
> > > > > No performance degradation was seen (fio libaio engine with 16 jobs 
> > > > > and
> > > > > 128 iodepth):
> > > > > 
> > > > > IO size  IOPs Rand Read (before/after) IOPs Rand Write 
> > > > > (before/after)
> > > > >  -
> > > > > --
> > > > > 512B  318K/316K329K/325K
> > > > > 
> > > > > 4KB   323K/321K353K/349K
> > > > > 
> > > > > 16KB  199K/208K250K/275K
> > > > > 
> > > > > 128KB 36K/36.1K39.2K/41.7K
> > > > I ran fio randread benchmarks with 4k, 16k, 64k, and 128k at iodepth 1,
> > > > 8, and 64 on two vCPUs. The results look fine, there is no significant
> > > > regression.
> > > > 
> > > > iodepth=1 and iodepth=64 are very consistent. For some reason the
> > > > iodepth=8 has significant variance but I don't think it's the fault of
> > > > this patch.
> > > > 
> > > > Fio results and the Jupyter notebook export are available here (check
> > > > out benchmark.html to see the graphs):
> > > > 
> > > > https://gitlab.com/stefanha/virt-playbooks/-/tree/virtio-blk-sgl-allocation-benchmark/notebook
> > > > 
> > > > Guest:
> > > > - Fedora 34
> > > > - Linux v5.14
> > > > - 2 vCPUs (pinned), 4 GB RAM (single host NUMA node)
> > > > - 1 IOThread (pinned)
> > > > - virtio-blk aio=native,cache=none,format=raw
> > > > - QEMU 6.1.0
> > > > 
> > > > Host:
> > > > - RHEL 8.3
> > > > - Linux 4.18.0-240.22.1.el8_3.x86_64
> > > > - Intel(R) Xeon(R) Silver 4214 CPU @ 2.20GHz
> > > > - Intel Optane DC P4800X
> > > > 
> > > > Stefan
> > > Thanks, Stefan.
> > > 
> > > Would you like me to add some of the results in my commit msg ? or 
> > > Tested-By
> > > sign ?
> > Thanks, there's no need to change the commit description.
> > 
> > Reviewed-by: Stefan Hajnoczi 
> > Tested-by: Stefan Hajnoczi 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] i2c: virtio: Add support for zero-length requests

2021-10-22 Thread Michael S. Tsirkin
On Fri, Oct 22, 2021 at 02:51:10PM +0800, Jie Deng wrote:
> 
> On 2021/10/21 17:47, Viresh Kumar wrote:
> > The virtio specification received a new mandatory feature
> > (VIRTIO_I2C_F_ZERO_LENGTH_REQUEST) for zero length requests. Fail if the
> > feature isn't offered by the device.
> > 
> > For each read-request, set the VIRTIO_I2C_FLAGS_M_RD flag, as required
> > by the VIRTIO_I2C_F_ZERO_LENGTH_REQUEST feature.
> > 
> > This allows us to support zero length requests, like SMBUS Quick, where
> > the buffer need not be sent anymore.
> > 
> > Signed-off-by: Viresh Kumar 
> > ---
> > Hi Wolfram,
> > 
> > Please do not apply this until the spec changes [1] are merged, sending it 
> > early
> > to get review done. I will ping you later once the spec is merged.
> > 
> > [1] https://lists.oasis-open.org/archives/virtio-dev/202110/msg00109.html
> > 
> >   drivers/i2c/busses/i2c-virtio.c | 56 ++---
> >   include/uapi/linux/virtio_i2c.h |  6 
> >   2 files changed, 36 insertions(+), 26 deletions(-)
> > 
> 
> Acked-by: Jie Deng  once the spec is merged.


There's supposed to be space before < btw. and one puts # before any
comments this way tools can process the ack automatically:

Acked-by: Jie Deng # once the spec is merged.

> 
> 
> > +   if (!virtio_has_feature(vdev, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST)) {
> > +   dev_err(>dev, "Zero-length request feature is 
> > mandatory\n");
> > +   return -EINVAL;
> 
> 
> It might be better to return -EOPNOTSUPP ?
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] i2c: virtio: Add support for zero-length requests

2021-10-22 Thread Viresh Kumar
On 22-10-21, 14:51, Jie Deng wrote:
> > +   if (!virtio_has_feature(vdev, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST)) {
> > +   dev_err(>dev, "Zero-length request feature is 
> > mandatory\n");
> > +   return -EINVAL;
> 
> 
> It might be better to return -EOPNOTSUPP ?

Maybe that or one of these:

#define EBADE   52  /* Invalid exchange */
#define EPROTO  71  /* Protocol error */
#define EPFNOSUPPORT96  /* Protocol family not supported */
#define ECONNREFUSED111 /* Connection refused */

Arnd, any suggestions ? This is about the mandatory feature not being offered by
the device.

-- 
viresh
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] i2c: virtio: Add support for zero-length requests

2021-10-22 Thread Jie Deng



On 2021/10/21 17:47, Viresh Kumar wrote:

The virtio specification received a new mandatory feature
(VIRTIO_I2C_F_ZERO_LENGTH_REQUEST) for zero length requests. Fail if the
feature isn't offered by the device.

For each read-request, set the VIRTIO_I2C_FLAGS_M_RD flag, as required
by the VIRTIO_I2C_F_ZERO_LENGTH_REQUEST feature.

This allows us to support zero length requests, like SMBUS Quick, where
the buffer need not be sent anymore.

Signed-off-by: Viresh Kumar 
---
Hi Wolfram,

Please do not apply this until the spec changes [1] are merged, sending it early
to get review done. I will ping you later once the spec is merged.

[1] https://lists.oasis-open.org/archives/virtio-dev/202110/msg00109.html

  drivers/i2c/busses/i2c-virtio.c | 56 ++---
  include/uapi/linux/virtio_i2c.h |  6 
  2 files changed, 36 insertions(+), 26 deletions(-)



Acked-by: Jie Deng  once the spec is merged.


  
+	if (!virtio_has_feature(vdev, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST)) {

+   dev_err(>dev, "Zero-length request feature is 
mandatory\n");
+   return -EINVAL;



It might be better to return -EOPNOTSUPP ?


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V3 00/11] vhost: multiple worker support

2021-10-22 Thread michael . christie
On 10/22/21 12:18 AM, Mike Christie wrote:
> Results:
> 
> 
> fio jobs1   2   4   8   12  16
> --
> 1 worker84k492k510k-   -   -

That should be 184k above.

> worker per vq   184k   380k744k1422k   2256k   2434k

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization