| Message ID | 20240723114914.53677-1-slp@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/virtio: introduce the HOST_PAGE_SIZE feature | expand |
On 7/23/24 14:49, Sergio Lopez wrote: > There's an incresing number of machines supporting multiple page sizes > and on these machines the host and a guest can be running, each one, > with a different page size. > > For what pertains to virtio-gpu, this is not a problem if the page size > of the guest happens to be bigger or equal than the host, but will > potentially lead to failures in memory allocations and/or mappings > otherwise. Please describe concrete problem you're trying to solve. Guest memory allocation consists of guest pages, I don't see how knowledge of host page size helps anything in userspace. I suspect you want this for host blobs, but then it should be virtio_gpu_vram_create() that should use max(host_page_sz, guest_page_size), AFAICT. It's kernel who is responsible for memory management, userspace can't be trusted for doing that.
Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: > On 7/23/24 14:49, Sergio Lopez wrote: >> There's an incresing number of machines supporting multiple page sizes >> and on these machines the host and a guest can be running, each one, >> with a different page size. >> >> For what pertains to virtio-gpu, this is not a problem if the page size >> of the guest happens to be bigger or equal than the host, but will >> potentially lead to failures in memory allocations and/or mappings >> otherwise. > > Please describe concrete problem you're trying to solve. Guest memory > allocation consists of guest pages, I don't see how knowledge of host > page size helps anything in userspace. > > I suspect you want this for host blobs, but then it should be > virtio_gpu_vram_create() that should use max(host_page_sz, > guest_page_size), AFAICT. It's kernel who is responsible for memory > management, userspace can't be trusted for doing that. Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation and mapping into the guest of device-backed memory and shmem regions. The CREATE_BLOB ioctl doesn't update drm_virtgpu_resource_create->size, so the guest kernel (and, as a consequence, the host kernel) can't override the user's request. I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the host page size to align the size of the CREATE_BLOB requests as required. Thanks, Sergio.
On Wed, Jul 24, 2024 at 12:00 PM Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > > On 7/23/24 14:49, Sergio Lopez wrote: > > There's an incresing number of machines supporting multiple page sizes > > and on these machines the host and a guest can be running, each one, > > with a different page size. > > > > For what pertains to virtio-gpu, this is not a problem if the page size > > of the guest happens to be bigger or equal than the host, but will > > potentially lead to failures in memory allocations and/or mappings > > otherwise. > > Please describe concrete problem you're trying to solve. Guest memory > allocation consists of guest pages, I don't see how knowledge of host > page size helps anything in userspace. > > I suspect you want this for host blobs, but then it should be > virtio_gpu_vram_create() that should use max(host_page_sz, > guest_page_size), AFAICT. It's kernel who is responsible for memory > management, userspace can't be trusted for doing that. fwiw virtgpu native context would require this as well, mesa would need to know the host page size to correctly align GPU VA allocations (which must be a multiple of the host page size). So a-b for adding this and exposing it to userspace. BR, -R > -- > Best regards, > Dmitry >
On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <slp@redhat.com> wrote: > Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: > > > On 7/23/24 14:49, Sergio Lopez wrote: > >> There's an incresing number of machines supporting multiple page sizes > >> and on these machines the host and a guest can be running, each one, > >> with a different page size. > >> > >> For what pertains to virtio-gpu, this is not a problem if the page size > >> of the guest happens to be bigger or equal than the host, but will > >> potentially lead to failures in memory allocations and/or mappings > >> otherwise. > > > > Please describe concrete problem you're trying to solve. Guest memory > > allocation consists of guest pages, I don't see how knowledge of host > > page size helps anything in userspace. > > > > I suspect you want this for host blobs, but then it should be > > virtio_gpu_vram_create() that should use max(host_page_sz, > > guest_page_size), AFAICT. It's kernel who is responsible for memory > > management, userspace can't be trusted for doing that. > > Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation > and mapping into the guest of device-backed memory and shmem regions. > The CREATE_BLOB ioctl doesn't update drm_virtgpu_resource_create->size, > so the guest kernel (and, as a consequence, the host kernel) can't > override the user's request. > > I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the host > page size to align the size of the CREATE_BLOB requests as required. > gfxstream solves this problem by putting the relevant information in the capabilities obtained from the host: https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 If you want to be paranoid, you can also validate the ResourceCreateBlob::size is properly host-page aligned when that request reaches the host. So you can probably solve this problem using current interfaces. Whether it's cleaner for all context types to use the capabilities, or have all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the cost/benefit tradeoff. > > Thanks, > Sergio. > >
On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh <gurchetansingh@chromium.org> wrote: > > > > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <slp@redhat.com> wrote: >> >> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: >> >> > On 7/23/24 14:49, Sergio Lopez wrote: >> >> There's an incresing number of machines supporting multiple page sizes >> >> and on these machines the host and a guest can be running, each one, >> >> with a different page size. >> >> >> >> For what pertains to virtio-gpu, this is not a problem if the page size >> >> of the guest happens to be bigger or equal than the host, but will >> >> potentially lead to failures in memory allocations and/or mappings >> >> otherwise. >> > >> > Please describe concrete problem you're trying to solve. Guest memory >> > allocation consists of guest pages, I don't see how knowledge of host >> > page size helps anything in userspace. >> > >> > I suspect you want this for host blobs, but then it should be >> > virtio_gpu_vram_create() that should use max(host_page_sz, >> > guest_page_size), AFAICT. It's kernel who is responsible for memory >> > management, userspace can't be trusted for doing that. >> >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation >> and mapping into the guest of device-backed memory and shmem regions. >> The CREATE_BLOB ioctl doesn't update drm_virtgpu_resource_create->size, >> so the guest kernel (and, as a consequence, the host kernel) can't >> override the user's request. >> >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the host >> page size to align the size of the CREATE_BLOB requests as required. > > > gfxstream solves this problem by putting the relevant information in the capabilities obtained from the host: > > https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 > > If you want to be paranoid, you can also validate the ResourceCreateBlob::size is properly host-page aligned when that request reaches the host. > > So you can probably solve this problem using current interfaces. Whether it's cleaner for all context types to use the capabilities, or have all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the cost/benefit tradeoff. > I guess solving it in a context-type specific way is possible. But I think it is a relatively universal constraint. And maybe it makes sense for virtgpu guest kernel to enforce alignment (at least it can return an error synchronously) in addition to the host. BR, -R >> >> >> Thanks, >> Sergio. >>
On Tue, Aug 6, 2024 at 1:15 PM Rob Clark <robdclark@gmail.com> wrote: > On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh > <gurchetansingh@chromium.org> wrote: > > > > > > > > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <slp@redhat.com> > wrote: > >> > >> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: > >> > >> > On 7/23/24 14:49, Sergio Lopez wrote: > >> >> There's an incresing number of machines supporting multiple page > sizes > >> >> and on these machines the host and a guest can be running, each one, > >> >> with a different page size. > >> >> > >> >> For what pertains to virtio-gpu, this is not a problem if the page > size > >> >> of the guest happens to be bigger or equal than the host, but will > >> >> potentially lead to failures in memory allocations and/or mappings > >> >> otherwise. > >> > > >> > Please describe concrete problem you're trying to solve. Guest memory > >> > allocation consists of guest pages, I don't see how knowledge of host > >> > page size helps anything in userspace. > >> > > >> > I suspect you want this for host blobs, but then it should be > >> > virtio_gpu_vram_create() that should use max(host_page_sz, > >> > guest_page_size), AFAICT. It's kernel who is responsible for memory > >> > management, userspace can't be trusted for doing that. > >> > >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation > >> and mapping into the guest of device-backed memory and shmem regions. > >> The CREATE_BLOB ioctl doesn't update drm_virtgpu_resource_create->size, > >> so the guest kernel (and, as a consequence, the host kernel) can't > >> override the user's request. > >> > >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the host > >> page size to align the size of the CREATE_BLOB requests as required. > > > > > > gfxstream solves this problem by putting the relevant information in the > capabilities obtained from the host: > > > > > https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 > > > > If you want to be paranoid, you can also validate the > ResourceCreateBlob::size is properly host-page aligned when that request > reaches the host. > > > > So you can probably solve this problem using current interfaces. > Whether it's cleaner for all context types to use the capabilities, or have > all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the cost/benefit > tradeoff. > > > > I guess solving it in a context-type specific way is possible. But I > think it is a relatively universal constraint. And maybe it makes > sense for virtgpu guest kernel to enforce alignment (at least it can > return an error synchronously) in addition to the host. > virtio-media may have support for VIRTIO_MEDIA_CMD_MMAP too, so could run into this issue. https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions virtio-fs also has the DAX window which uses the same memory mapping mechanism. https://virtio-fs.gitlab.io/design.html Maybe this should not be a virtio-gpu thing, but a virtio thing? > > BR, > -R > > >> > >> > >> Thanks, > >> Sergio. > >> >
Gurchetan Singh <gurchetansingh@chromium.org> writes: > On Tue, Aug 6, 2024 at 1:15 PM Rob Clark <robdclark@gmail.com> wrote: > >> On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh >> <gurchetansingh@chromium.org> wrote: >> > >> > >> > >> > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <slp@redhat.com> >> wrote: >> >> >> >> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: >> >> >> >> > On 7/23/24 14:49, Sergio Lopez wrote: >> >> >> There's an incresing number of machines supporting multiple page >> sizes >> >> >> and on these machines the host and a guest can be running, each one, >> >> >> with a different page size. >> >> >> >> >> >> For what pertains to virtio-gpu, this is not a problem if the page >> size >> >> >> of the guest happens to be bigger or equal than the host, but will >> >> >> potentially lead to failures in memory allocations and/or mappings >> >> >> otherwise. >> >> > >> >> > Please describe concrete problem you're trying to solve. Guest memory >> >> > allocation consists of guest pages, I don't see how knowledge of host >> >> > page size helps anything in userspace. >> >> > >> >> > I suspect you want this for host blobs, but then it should be >> >> > virtio_gpu_vram_create() that should use max(host_page_sz, >> >> > guest_page_size), AFAICT. It's kernel who is responsible for memory >> >> > management, userspace can't be trusted for doing that. >> >> >> >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation >> >> and mapping into the guest of device-backed memory and shmem regions. >> >> The CREATE_BLOB ioctl doesn't update drm_virtgpu_resource_create->size, >> >> so the guest kernel (and, as a consequence, the host kernel) can't >> >> override the user's request. >> >> >> >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the host >> >> page size to align the size of the CREATE_BLOB requests as required. >> > >> > >> > gfxstream solves this problem by putting the relevant information in the >> capabilities obtained from the host: >> > >> > >> https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 >> > >> > If you want to be paranoid, you can also validate the >> ResourceCreateBlob::size is properly host-page aligned when that request >> reaches the host. >> > >> > So you can probably solve this problem using current interfaces. >> Whether it's cleaner for all context types to use the capabilities, or have >> all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the cost/benefit >> tradeoff. >> > >> >> I guess solving it in a context-type specific way is possible. But I >> think it is a relatively universal constraint. And maybe it makes >> sense for virtgpu guest kernel to enforce alignment (at least it can >> return an error synchronously) in addition to the host. >> > > virtio-media may have support for VIRTIO_MEDIA_CMD_MMAP too, so could run > into this issue. > > https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions > > virtio-fs also has the DAX window which uses the same memory mapping > mechanism. > > https://virtio-fs.gitlab.io/design.html > > Maybe this should not be a virtio-gpu thing, but a virtio thing? This is true, but finding a common place to put the page size is really hard in practice. I don't think we can borrow space in the feature bits for that (and that would probably be abusing its purpose quite a bit) and extending the transport configuration registers is quite cumbersome and, in general, undesirable. That leaves us with the device-specific config space, and that implies a device-specific feature bit as it's implemented in this series. The Shared Memory Regions on the VIRTIO spec, while doesn't talk specifically about page size, also gives us a hint about this being the right direction: " 2.10 Shared Memory Regions (...) Memory consistency rules vary depending on the region and the device and they will be specified as required by each device." " Thanks, Sergio.
On 8/5/24 19:24, Rob Clark wrote: > On Wed, Jul 24, 2024 at 12:00 PM Dmitry Osipenko > <dmitry.osipenko@collabora.com> wrote: >> >> On 7/23/24 14:49, Sergio Lopez wrote: >>> There's an incresing number of machines supporting multiple page sizes >>> and on these machines the host and a guest can be running, each one, >>> with a different page size. >>> >>> For what pertains to virtio-gpu, this is not a problem if the page size >>> of the guest happens to be bigger or equal than the host, but will >>> potentially lead to failures in memory allocations and/or mappings >>> otherwise. >> >> Please describe concrete problem you're trying to solve. Guest memory >> allocation consists of guest pages, I don't see how knowledge of host >> page size helps anything in userspace. >> >> I suspect you want this for host blobs, but then it should be >> virtio_gpu_vram_create() that should use max(host_page_sz, >> guest_page_size), AFAICT. It's kernel who is responsible for memory >> management, userspace can't be trusted for doing that. > > fwiw virtgpu native context would require this as well, mesa would > need to know the host page size to correctly align GPU VA allocations > (which must be a multiple of the host page size). > > So a-b for adding this and exposing it to userspace. In general, GPU page size has no connection to the CPU page size. It happens that MSM driver uses same page size for both GPU and CPU. Likely you could configure a different GPU page size if you wanted. dGPUs would often use 64k pages.
On Thu, Aug 8, 2024 at 3:38 AM Sergio Lopez Pascual <slp@redhat.com> wrote: > Gurchetan Singh <gurchetansingh@chromium.org> writes: > > > On Tue, Aug 6, 2024 at 1:15 PM Rob Clark <robdclark@gmail.com> wrote: > > > >> On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh > >> <gurchetansingh@chromium.org> wrote: > >> > > >> > > >> > > >> > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <slp@redhat.com> > >> wrote: > >> >> > >> >> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: > >> >> > >> >> > On 7/23/24 14:49, Sergio Lopez wrote: > >> >> >> There's an incresing number of machines supporting multiple page > >> sizes > >> >> >> and on these machines the host and a guest can be running, each > one, > >> >> >> with a different page size. > >> >> >> > >> >> >> For what pertains to virtio-gpu, this is not a problem if the page > >> size > >> >> >> of the guest happens to be bigger or equal than the host, but will > >> >> >> potentially lead to failures in memory allocations and/or mappings > >> >> >> otherwise. > >> >> > > >> >> > Please describe concrete problem you're trying to solve. Guest > memory > >> >> > allocation consists of guest pages, I don't see how knowledge of > host > >> >> > page size helps anything in userspace. > >> >> > > >> >> > I suspect you want this for host blobs, but then it should be > >> >> > virtio_gpu_vram_create() that should use max(host_page_sz, > >> >> > guest_page_size), AFAICT. It's kernel who is responsible for memory > >> >> > management, userspace can't be trusted for doing that. > >> >> > >> >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation > >> >> and mapping into the guest of device-backed memory and shmem regions. > >> >> The CREATE_BLOB ioctl doesn't update > drm_virtgpu_resource_create->size, > >> >> so the guest kernel (and, as a consequence, the host kernel) can't > >> >> override the user's request. > >> >> > >> >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the > host > >> >> page size to align the size of the CREATE_BLOB requests as required. > >> > > >> > > >> > gfxstream solves this problem by putting the relevant information in > the > >> capabilities obtained from the host: > >> > > >> > > >> > https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 > >> > > >> > If you want to be paranoid, you can also validate the > >> ResourceCreateBlob::size is properly host-page aligned when that request > >> reaches the host. > >> > > >> > So you can probably solve this problem using current interfaces. > >> Whether it's cleaner for all context types to use the capabilities, or > have > >> all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the > cost/benefit > >> tradeoff. > >> > > >> > >> I guess solving it in a context-type specific way is possible. But I > >> think it is a relatively universal constraint. And maybe it makes > >> sense for virtgpu guest kernel to enforce alignment (at least it can > >> return an error synchronously) in addition to the host. > >> > > > > virtio-media may have support for VIRTIO_MEDIA_CMD_MMAP too, so could run > > into this issue. > > > > > https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions > > > > virtio-fs also has the DAX window which uses the same memory mapping > > mechanism. > > > > https://virtio-fs.gitlab.io/design.html > > > > Maybe this should not be a virtio-gpu thing, but a virtio thing? > > This is true, but finding a common place to put the page size is really > hard in practice. I don't think we can borrow space in the feature bits > for that (and that would probably be abusing its purpose quite a bit) > and extending the transport configuration registers is quite cumbersome > and, in general, undesirable. > > That leaves us with the device-specific config space, and that implies a > device-specific feature bit as it's implemented in this series. > > The Shared Memory Regions on the VIRTIO spec, while doesn't talk > specifically about page size, also gives us a hint about this being the > right direction: > Can't we just modify the Shared Memory region PCI capability to include page size? We can either: 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. or 2) Or expand the size of the struct, with VIRTIO_PCI_CAP_SHARED_MEMORY_CFG2. (sketch here: crrev.com/c/5778179) 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. > > " > 2.10 Shared Memory Regions > (...) > Memory consistency rules vary depending on the region and the device > and they will be specified as required by each device." > " > > Thanks, > Sergio. > >
On Thu, Aug 8, 2024 at 4:16 AM Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > > On 8/5/24 19:24, Rob Clark wrote: > > On Wed, Jul 24, 2024 at 12:00 PM Dmitry Osipenko > > <dmitry.osipenko@collabora.com> wrote: > >> > >> On 7/23/24 14:49, Sergio Lopez wrote: > >>> There's an incresing number of machines supporting multiple page sizes > >>> and on these machines the host and a guest can be running, each one, > >>> with a different page size. > >>> > >>> For what pertains to virtio-gpu, this is not a problem if the page size > >>> of the guest happens to be bigger or equal than the host, but will > >>> potentially lead to failures in memory allocations and/or mappings > >>> otherwise. > >> > >> Please describe concrete problem you're trying to solve. Guest memory > >> allocation consists of guest pages, I don't see how knowledge of host > >> page size helps anything in userspace. > >> > >> I suspect you want this for host blobs, but then it should be > >> virtio_gpu_vram_create() that should use max(host_page_sz, > >> guest_page_size), AFAICT. It's kernel who is responsible for memory > >> management, userspace can't be trusted for doing that. > > > > fwiw virtgpu native context would require this as well, mesa would > > need to know the host page size to correctly align GPU VA allocations > > (which must be a multiple of the host page size). > > > > So a-b for adding this and exposing it to userspace. > > In general, GPU page size has no connection to the CPU page size. It > happens that MSM driver uses same page size for both GPU and CPU. Likely > you could configure a different GPU page size if you wanted. dGPUs would > often use 64k pages. The smmu actually supports various different page sizes (4k, 64k, etc.. I think up to 2g), and will try to map larger contiguous sets of pages using larger page sizes to reduce TLB pressure. This restriction about aligning to host page size is because the kernel expects allocations and therefore (currently, pre-sparse) gpu mappings to be a multiple of the host page size. As far as whether this should be something outside of virtio-gpu, this does feel a bit specific to how GEM buffer allocations work and how host blob resources work. Maybe other subsystems like media end up with similar constraints for similar reasons, idk. But it at least feels like something applicable to all/most virtgpu context types. BR, -R
Gurchetan Singh <gurchetansingh@chromium.org> writes: > On Thu, Aug 8, 2024 at 3:38 AM Sergio Lopez Pascual <slp@redhat.com> wrote: > >> Gurchetan Singh <gurchetansingh@chromium.org> writes: >> >> > On Tue, Aug 6, 2024 at 1:15 PM Rob Clark <robdclark@gmail.com> wrote: >> > >> >> On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh >> >> <gurchetansingh@chromium.org> wrote: >> >> > >> >> > >> >> > >> >> > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual <slp@redhat.com> >> >> wrote: >> >> >> >> >> >> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: >> >> >> >> >> >> > On 7/23/24 14:49, Sergio Lopez wrote: >> >> >> >> There's an incresing number of machines supporting multiple page >> >> sizes >> >> >> >> and on these machines the host and a guest can be running, each >> one, >> >> >> >> with a different page size. >> >> >> >> >> >> >> >> For what pertains to virtio-gpu, this is not a problem if the page >> >> size >> >> >> >> of the guest happens to be bigger or equal than the host, but will >> >> >> >> potentially lead to failures in memory allocations and/or mappings >> >> >> >> otherwise. >> >> >> > >> >> >> > Please describe concrete problem you're trying to solve. Guest >> memory >> >> >> > allocation consists of guest pages, I don't see how knowledge of >> host >> >> >> > page size helps anything in userspace. >> >> >> > >> >> >> > I suspect you want this for host blobs, but then it should be >> >> >> > virtio_gpu_vram_create() that should use max(host_page_sz, >> >> >> > guest_page_size), AFAICT. It's kernel who is responsible for memory >> >> >> > management, userspace can't be trusted for doing that. >> >> >> >> >> >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the creation >> >> >> and mapping into the guest of device-backed memory and shmem regions. >> >> >> The CREATE_BLOB ioctl doesn't update >> drm_virtgpu_resource_create->size, >> >> >> so the guest kernel (and, as a consequence, the host kernel) can't >> >> >> override the user's request. >> >> >> >> >> >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the >> host >> >> >> page size to align the size of the CREATE_BLOB requests as required. >> >> > >> >> > >> >> > gfxstream solves this problem by putting the relevant information in >> the >> >> capabilities obtained from the host: >> >> > >> >> > >> >> >> https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 >> >> > >> >> > If you want to be paranoid, you can also validate the >> >> ResourceCreateBlob::size is properly host-page aligned when that request >> >> reaches the host. >> >> > >> >> > So you can probably solve this problem using current interfaces. >> >> Whether it's cleaner for all context types to use the capabilities, or >> have >> >> all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the >> cost/benefit >> >> tradeoff. >> >> > >> >> >> >> I guess solving it in a context-type specific way is possible. But I >> >> think it is a relatively universal constraint. And maybe it makes >> >> sense for virtgpu guest kernel to enforce alignment (at least it can >> >> return an error synchronously) in addition to the host. >> >> >> > >> > virtio-media may have support for VIRTIO_MEDIA_CMD_MMAP too, so could run >> > into this issue. >> > >> > >> https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions >> > >> > virtio-fs also has the DAX window which uses the same memory mapping >> > mechanism. >> > >> > https://virtio-fs.gitlab.io/design.html >> > >> > Maybe this should not be a virtio-gpu thing, but a virtio thing? >> >> This is true, but finding a common place to put the page size is really >> hard in practice. I don't think we can borrow space in the feature bits >> for that (and that would probably be abusing its purpose quite a bit) >> and extending the transport configuration registers is quite cumbersome >> and, in general, undesirable. >> >> That leaves us with the device-specific config space, and that implies a >> device-specific feature bit as it's implemented in this series. >> >> The Shared Memory Regions on the VIRTIO spec, while doesn't talk >> specifically about page size, also gives us a hint about this being the >> right direction: >> > > Can't we just modify the Shared Memory region PCI capability to include > page size? We can either: > > 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. Yes, we can turn that padding into "__le16 page_size_order" to store "PAGE_SIZE >> 12". That should be enough to secure some future-proofing. There's also some space in the "MMIO Device Register Layout" to store it as a 16 bit or 32 bit value. This would require proposing it as a change to the VIRTIO specs. Do you want to do it yourself or should I take the initiative? Thanks, Sergio.
On Thu, Aug 22, 2024 at 8:29 AM Sergio Lopez Pascual <slp@redhat.com> wrote: > Gurchetan Singh <gurchetansingh@chromium.org> writes: > > > On Thu, Aug 8, 2024 at 3:38 AM Sergio Lopez Pascual <slp@redhat.com> > wrote: > > > >> Gurchetan Singh <gurchetansingh@chromium.org> writes: > >> > >> > On Tue, Aug 6, 2024 at 1:15 PM Rob Clark <robdclark@gmail.com> wrote: > >> > > >> >> On Tue, Aug 6, 2024 at 9:15 AM Gurchetan Singh > >> >> <gurchetansingh@chromium.org> wrote: > >> >> > > >> >> > > >> >> > > >> >> > On Mon, Aug 5, 2024 at 2:14 AM Sergio Lopez Pascual < > slp@redhat.com> > >> >> wrote: > >> >> >> > >> >> >> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: > >> >> >> > >> >> >> > On 7/23/24 14:49, Sergio Lopez wrote: > >> >> >> >> There's an incresing number of machines supporting multiple > page > >> >> sizes > >> >> >> >> and on these machines the host and a guest can be running, each > >> one, > >> >> >> >> with a different page size. > >> >> >> >> > >> >> >> >> For what pertains to virtio-gpu, this is not a problem if the > page > >> >> size > >> >> >> >> of the guest happens to be bigger or equal than the host, but > will > >> >> >> >> potentially lead to failures in memory allocations and/or > mappings > >> >> >> >> otherwise. > >> >> >> > > >> >> >> > Please describe concrete problem you're trying to solve. Guest > >> memory > >> >> >> > allocation consists of guest pages, I don't see how knowledge of > >> host > >> >> >> > page size helps anything in userspace. > >> >> >> > > >> >> >> > I suspect you want this for host blobs, but then it should be > >> >> >> > virtio_gpu_vram_create() that should use max(host_page_sz, > >> >> >> > guest_page_size), AFAICT. It's kernel who is responsible for > memory > >> >> >> > management, userspace can't be trusted for doing that. > >> >> >> > >> >> >> Mesa's Vulkan/Venus uses CREATE_BLOB to request the host the > creation > >> >> >> and mapping into the guest of device-backed memory and shmem > regions. > >> >> >> The CREATE_BLOB ioctl doesn't update > >> drm_virtgpu_resource_create->size, > >> >> >> so the guest kernel (and, as a consequence, the host kernel) can't > >> >> >> override the user's request. > >> >> >> > >> >> >> I'd like Mesa's Vulkan/Venus in the guest to be able to obtain the > >> host > >> >> >> page size to align the size of the CREATE_BLOB requests as > required. > >> >> > > >> >> > > >> >> > gfxstream solves this problem by putting the relevant information > in > >> the > >> >> capabilities obtained from the host: > >> >> > > >> >> > > >> >> > >> > https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/heads/main/host/virtio-gpu-gfxstream-renderer.cpp#1691 > >> >> > > >> >> > If you want to be paranoid, you can also validate the > >> >> ResourceCreateBlob::size is properly host-page aligned when that > request > >> >> reaches the host. > >> >> > > >> >> > So you can probably solve this problem using current interfaces. > >> >> Whether it's cleaner for all context types to use the capabilities, > or > >> have > >> >> all VMMs to expose VIRTIO_GPU_F_HOST_PAGE_SIZE, would be the > >> cost/benefit > >> >> tradeoff. > >> >> > > >> >> > >> >> I guess solving it in a context-type specific way is possible. But I > >> >> think it is a relatively universal constraint. And maybe it makes > >> >> sense for virtgpu guest kernel to enforce alignment (at least it can > >> >> return an error synchronously) in addition to the host. > >> >> > >> > > >> > virtio-media may have support for VIRTIO_MEDIA_CMD_MMAP too, so could > run > >> > into this issue. > >> > > >> > > >> > https://github.com/chromeos/virtio-media?tab=readme-ov-file#shared-memory-regions > >> > > >> > virtio-fs also has the DAX window which uses the same memory mapping > >> > mechanism. > >> > > >> > https://virtio-fs.gitlab.io/design.html > >> > > >> > Maybe this should not be a virtio-gpu thing, but a virtio thing? > >> > >> This is true, but finding a common place to put the page size is really > >> hard in practice. I don't think we can borrow space in the feature bits > >> for that (and that would probably be abusing its purpose quite a bit) > >> and extending the transport configuration registers is quite cumbersome > >> and, in general, undesirable. > >> > >> That leaves us with the device-specific config space, and that implies a > >> device-specific feature bit as it's implemented in this series. > >> > >> The Shared Memory Regions on the VIRTIO spec, while doesn't talk > >> specifically about page size, also gives us a hint about this being the > >> right direction: > >> > > > > Can't we just modify the Shared Memory region PCI capability to include > > page size? We can either: > > > > 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. > > Yes, we can turn that padding into "__le16 page_size_order" to store > "PAGE_SIZE >> 12". That should be enough to secure some future-proofing. > There's also some space in the "MMIO Device Register Layout" to store it > as a 16 bit or 32 bit value. > > This would require proposing it as a change to the VIRTIO specs. Do you > want to do it yourself or should I take the initiative? > You should do it. > > Thanks, > Sergio. > >