<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Aug 8, 2024 at 3:38 AM Sergio Lopez Pascual <<a href="mailto:slp@redhat.com">slp@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Gurchetan Singh <<a href="mailto:gurchetansingh@chromium.org" target="_blank">gurchetansingh@chromium.org</a>> writes:<br>
<br>
> On Tue, Aug 6, 2024 at 1:15 PM Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> wrote:<br>
><br>
>> On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh<br>
>> <<a href="mailto:gurchetansingh@chromium.org" target="_blank">gurchetansingh@chromium.org</a>> wrote:<br>
>> ><br>
>> ><br>
>> ><br>
>> > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <<a href="mailto:slp@redhat.com" target="_blank">slp@redhat.com</a>><br>
>> wrote:<br>
>> >><br>
>> >> Dmitry Osipenko <<a href="mailto:dmitry.osipenko@collabora.com" target="_blank">dmitry.osipenko@collabora.com</a>> writes:<br>
>> >><br>
>> >> > On 7/23/24 14:49, Sergio Lopez wrote:<br>
>> >> >> There's an incresing number of machines supporting multiple page<br>
>> sizes<br>
>> >> >> and on these machines the host and a guest can be running, each one,<br>
>> >> >> with a different page size.<br>
>> >> >><br>
>> >> >> For what pertains to virtio-gpu, this is not a problem if the page<br>
>> size<br>
>> >> >> of the guest happens to be bigger or equal than the host, but will<br>
>> >> >> potentially lead to failures in memory allocations and/or mappings<br>
>> >> >> otherwise.<br>
>> >> ><br>
>> >> > Please describe concrete problem you're trying to solve. Guest memory<br>
>> >> > allocation consists of guest pages, I don't see how knowledge of host<br>
>> >> > page size helps anything in userspace.<br>
>> >> ><br>
>> >> > I suspect you want this for host blobs, but then it should be<br>
>> >> > virtio_gpu_vram_create() that should use max(host_page_sz,<br>
>> >> > guest_page_size), AFAICT. It's kernel who is responsible for memory<br>
>> >> > management, userspace can't be trusted for doing that.<br>
>> >><br>
>> >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation<br>
>> >> and mapping into the guest of device-backed memory and shmem regions.<br>
>> >> The CREATE_BLOB ioctl doesn't update drm_virtgpu_resource_create->size,<br>
>> >> so the guest kernel (and, as a consequence, the host kernel) can't<br>
>> >> override the user's request.<br>
>> >><br>
>> >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the host<br>
>> >> page size to align the size of the CREATE_BLOB requests as required.<br>
>> ><br>
>> ><br>
>> > gfxstream solves this problem by putting the relevant information in the<br>
>> capabilities obtained from the host:<br>
>> ><br>
>> ><br>
>> <a href="https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691" rel="noreferrer" target="_blank">https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691</a><br>
>> ><br>
>> > If you want to be paranoid, you can also validate the<br>
>> ResourceCreateBlob::size is properly host-page aligned when that request<br>
>> reaches the host.<br>
>> ><br>
>> > So you can probably solve this problem using current interfaces.<br>
>> Whether it's cleaner for all context types to use the capabilities, or have<br>
>> all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the cost/benefit<br>
>> tradeoff.<br>
>> ><br>
>><br>
>> I guess solving it in a context-type specific way is possible.  But I<br>
>> think it is a relatively universal constraint.  And maybe it makes<br>
>> sense for virtgpu guest kernel to enforce alignment (at least it can<br>
>> return an error synchronously) in addition to the host.<br>
>><br>
><br>
> virtio-media may have support for VIRTIO_MEDIA_CMD_MMAP too, so could run<br>
> into this issue.<br>
><br>
> <a href="https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions" rel="noreferrer" target="_blank">https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions</a><br>
><br>
> virtio-fs also has the DAX window which uses the same memory mapping<br>
> mechanism.<br>
><br>
> <a href="https://virtio-fs.gitlab.io/design.html" rel="noreferrer" target="_blank">https://virtio-fs.gitlab.io/design.html</a><br>
><br>
> Maybe this should not be a virtio-gpu thing, but a virtio thing?<br>
<br>
This is true, but finding a common place to put the page size is really<br>
hard in practice. I don't think we can borrow space in the feature bits<br>
for that (and that would probably be abusing its purpose quite a bit)<br>
and extending the transport configuration registers is quite cumbersome<br>
and, in general, undesirable.<br>
<br>
That leaves us with the device-specific config space, and that implies a<br>
device-specific feature bit as it's implemented in this series.<br>
<br>
The Shared Memory Regions on the VIRTIO spec, while doesn't talk<br>
specifically about page size, also gives us a hint about this being the<br>
right direction:<br></blockquote><div><br></div><div>Can't we just modify the Shared Memory region PCI capability to include page size?  We can either:</div><div><br></div><div>1) keep the same size struct + header (VIRTIO_PCI_CAP_SHARED_MEMORY_CFG), and just hijack one of the padding fields. If the padding field is zero, we can just say it's 4096.</div><div><br></div><div>or</div><div> </div><div>2) Or expand the size of the struct, with VIRTIO_PCI_CAP_SHARED_MEMORY_CFG2. </div><div><br></div><div>(sketch here: <a href="http://crrev.com/c/5778179">crrev.com/c/5778179</a>)</div><div><br></div><div>The benefit of this would work with virtio-fs (though I'm not sure anyone uses the DAX window), and possibly virtio-media in the future.  </div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
"<br>
2.10 Shared Memory Regions<br>
(...)<br>
Memory consistency rules vary depending on the region and the device<br>
and they will be specified as required by each device."<br>
"<br>
<br>
Thanks,<br>
Sergio.<br>
<br>
</blockquote></div></div>