On Mon, Feb 07, 2022 at 04:05:16PM +0100, Halil Pasic wrote: > On Mon, 07 Feb 2022 14:41:58 +0100 > Cornelia Huck <coh...@redhat.com> wrote: > > > On Mon, Feb 07 2022, Daniel Henrique Barboza <danielhb...@gmail.com> wrote: > > > > > On 2/3/22 13:45, Halil Pasic wrote: > > >> Unlike most virtio features ACCESS_PATFORM is considered mandatory, i.e. > > >> > > > > s/ACCESS_PATFORM/ACCESS_PLATFORM/ > > Will fix. > > > > > >> the driver must accept it if offered by the device. The virtio > > >> specification says that the driver SHOULD accept the ACCESS_PLATFORM > > >> feature if offered, and that the device MAY fail to operate if > > >> ACCESS_PLATFORM was offered but not negotiated. > > >> > > >> While a SHOULD ain't exactly a MUST, we are certainly allowed to fail > > >> the device when the driver fences ACCESS_PLATFORM. With commit > > > > > > > > > I believe a link to the virtio specification where this is being > > > mentioned would > > > be good to have in the commit message. > > > > It's in section 6.1 "Driver Requirements: Reserved Feature Bits": "A > > driver SHOULD accept VIRTIO_F_ACCESS_PLATFORM if it is offered" and > > section 6.2 "Device Requirements: Reserved Feature Bits": "A device MAY > > fail to operate further if VIRTIO_F_ACCESS_PLATFORM is not accepted." > > > > That said, I'm not sure the wording in the spec translates to > > "mandatory"... if the driver fails to accept the bit, the device can > > choose to not work with the driver, but it's not forced to. > > I didn't mean to claim that the spec makes this feature "mandatory", and > this is why I paraphrased the spec. IMHO it is QEMU that considers it > mandatory.
this: A device MAY fail to operate further if VIRTIO_F_ACCESS_PLATFORM is not accepted is the hint here. > > There are > > other instances where the device may reject FEATURES_OK (e.g. when the > > driver does not accept a feature that is a pre-req for another feature), > > I'd say it is up to the device whether something is mandatory or not. If > > the device/setup cannot work without it, it certainly is mandatory, but > > the driver only knows when FEATURES_OK is rejected without the feature. > > Right but for the guys that write the drivers it is of interest to know > what level of interoperability can one can keep if certain > features are > not implemented. Usually it is safe to fence delay implementing > features, as long as the support for the features is implemented in the > order mandated by the spec. > > > > > OTOH, the decision to make it mandatory is certainly sound, and covered > > by the spec. As the driver must be prepared for the device failing to > > accept FEATURES_OK, we can make it mandatory here -- we should just not > > say that it is considered mandatory from a spec standpoint. The spec > > allows to make it mandatory, and we make it mandatory in our > > implementation. > > Right. Was never my intention to say that it is considered mandatory > by the spec. I guess the spec considers it less optional than the > run of the mill features. It would be nice to have a security considerations section. The point is that within guest, with ACCESS_PLATFORM it should be safe to assume that device can be passed through to nested guests or userspace. > Should I change the first sentence to something like "Unlike most virtio > features ACCESS_PATFORM is considered mandatory by QEMU, i.e. the driver > must accept it if offered by the device." > > [..] > > Regards, > Halil