On 07.04.25 11:13, David Hildenbrand wrote:
On 07.04.25 11:11, David Hildenbrand wrote:
On 07.04.25 10:58, Michael S. Tsirkin wrote:
On Mon, Apr 07, 2025 at 10:54:00AM +0200, David Hildenbrand wrote:
On 07.04.25 10:49, Michael S. Tsirkin wrote:
On Mon, Apr 07, 2025 at 10:44:21AM +0200, David Hildenbrand wrote:



Whoever adds new feat_X *must be aware* about all previous features,
otherwise we'd be reusing feature bits and everything falls to pieces.


The knowledge is supposed be limited to which feature bit to use.

I think we also have to know which virtqueue bits can be used, right?


what are virtqueue bits? vq number?

Yes, sorry.

I got confused myself, it's vq index actually now, we made the spec
consistent with that terminology. used to be number/index
interchangeably.

Assume cross-vm as an example. It would make use of virtqueue indexes 5+6
with their VIRTIO_BALLOON_F_WS_REPORTING.


crossvm guys really should have reserved the feature bit even if they
did not bother specifying it. Let's reserve it now at least?

Along with the virtqueue indices, right?

Note that there was

https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg02503.html

and

https://groups.oasis-open.org/communities/community-home/digestviewer/viewthread?GroupId=3973&MessageKey=afb07613-f56c-4d40-8981-2fad1c723998&CommunityKey=2f26be99-3aa1-48f6-93a5-018dce262226&hlmlt=VT

But it only was RFC, and as the QEMU implementation didn't materialize,
nobody seemed to care ...

Heh, but that one said:

+\item[ VIRTIO_BALLOON_F_WS_REPORTING(6) ] The device has support for
Working Set

Which does not seem to reflect reality ...


I dug a bit more into cross-vm, because that one seems to be the only
one out there that does not behave like everybody else I found (maybe good,
maybe bad :) ).


1) There was temporarily even another feature (VIRTIO_BALLOON_F_EVENTS_VQ)
and another queue.

It got removed from cross-vm in:

commit 9ba634b82b55ba762dc8724676b2cf9419460145
Author: Daniel Verkamp <dverk...@chromium.org>
Date:   Thu Jul 11 11:29:52 2024 -0700

    devices: virtio-balloon: remove event queue support
VIRTIO_BALLOON_F_EVENTS_VQ was part of a proposed virtio spec change. It is not currently supported by upstream Linux, so removing this should
    have no effect except for guest kernels that had CHROMIUM patches
    applied.
The virtqueue indexes for the ws-related queues are decremented to fill
    the hole left by the removal of the event VQ; these are non-standard as
    well, so they do not have virtqueue indexes assigned in the virtio spec,
    but the proposed spec extension did actually use vq indexes 5 and 6.
BUG=b:214864326


2) cross-vm is aware of the upstream Linux driver

They thought your fix would go upstream; it didn't.

commit a2fa119e759d0238a42ff15a9aff0dfd122afebd
Author: Daniel Verkamp <dverk...@chromium.org>
Date:   Wed Jul 10 16:16:28 2024 -0700

    devices: virtio-balloon: warn about queue index mismatches
The Linux kernel virtio-balloon driver spec non-compliance related to
    queue numbering is being fixed; add some diagnostics to our device that
    help to check if everything is working as expected.
<https://lore.kernel.org/virtualization/cacgkmesg0+vpav1fo8jf1isq4ef8t4_cfn1scyztdo8bxzr...@mail.gmail.com/T/> Additionally, replace the num_expected_queues() function with per-queue
    checking to avoid the need for the duplicate feature checks and queue
    count calculation; each pop_queue() call will be checked using the `?`
    operator and return a more useful error message if a particular queue is
    missing.
BUG=None
    TEST=crosvm run --balloon-page-reporting ...


IIRC, in that commit they switched to the "spec" behavior.

That's when they started hard-coding the queue indexes.

CCing Daniel. All Linux versions should be incompatible with cross-vmm 
regarding free page reporting.
How is that handled?

--
Cheers,

David / dhildenb


Reply via email to