Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220805181105.GA29848@willie-the-truck> <20220806074828.zwzgn5gj47gjx5og@sgarzare-redhat>
 <20220806094239.GA30268@willie-the-truck>
In-Reply-To: <20220806094239.GA30268@willie-the-truck>
From:   Stefan Hajnoczi <shajnocz@redhat.com>
Date:   Sat, 6 Aug 2022 06:52:15 -0400
Message-ID: <CAD60JZMbbkwFHqCm_iCrOrKgRLBUMkDQfuJ=Q1T-sZt59eTBrw@mail.gmail.com>
Subject: Re: IOTLB support for vhost/vsock breaks crosvm on Android
To:     Will Deacon <will@kernel.org>
Cc:     Stefano Garzarella <sgarzare@redhat.com>, mst@redhat.com,
        Stefan Hajnoczi <stefanha@redhat.com>,
        Jason Wang <jasowang@redhat.com>,
        torvalds@linux-foundation.org, ascull@google.com, maz@kernel.org,
        keirf@google.com, jiyong@google.com, kernel-team@android.com,
        linux-kernel@vger.kernel.org,
        virtualization@lists.linux-foundation.org, kvm@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Aug 6, 2022 at 5:50 AM Will Deacon <will@kernel.org> wrote:
> On Sat, Aug 06, 2022 at 09:48:28AM +0200, Stefano Garzarella wrote:
> > On Fri, Aug 05, 2022 at 07:11:06PM +0100, Will Deacon wrote:
> > If the VMM implements the translation feature, it is right in my opinion
> > that it does not enable the feature for the vhost device. Otherwise, if it
> > wants the vhost device to do the translation, enable the feature and send
> > the IOTLB messages to set the translation.
> >
> > QEMU for example masks features when not required or supported.
> > crosvm should negotiate only the features it supports.
> >
> > @Michael and @Jason can correct me, but if a vhost device negotiates
> > VIRTIO_F_ACCESS_PLATFORM, then it expects the VMM to send IOTLB messages to
> > set the translation.
>
> As above, the issue is that vhost now unconditionally advertises this in
> VHOST_GET_FEATURES and so a VMM with no knowledge of IOTLB can end up
> enabling it by accident.

Unconditionally exposing all vhost feature bits to the guest is
incorrect. The emulator must filter out only the feature bits that it
supports.

For example, see QEMU's vhost-net device's vhost feature bit allowlist:
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/net/vhost_net.c#L40

The reason why the emulator (crosvm/QEMU/etc) must filter out feature
bits is that vhost devices are full VIRTIO devices. They are a subset
of a VIRTIO device and the emulator is responsible for the rest of the
device. Some features will require both vhost and emulator support.
Therefore it is incorrect to expect the device to work correctly if
the vhost feature bits are passed through to the guest.

Stefan