[virtio-dev] Re: [virtio-comment] Re: [PATCH 09/11] transport-pci: Describe PCI MMR dev config registers

Yan Vugenfirer
2023-04-11T11:03:30Z

Sent from my iPhone

> On 11 Apr 2023, at 13:39, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> ﻿On Tue, Apr 11, 2023 at 12:19:14PM +0300, Yan Vugenfirer wrote:
>>> On Tue, Apr 11, 2023 at 12:02 PM Jason Wang <jasowang@redhat.com> wrote:
>>>
>>> On Tue, Apr 11, 2023 at 3:04 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>
>>>> On Tue, Apr 11, 2023 at 10:13:40AM +0800, Jason Wang wrote:
>>>>> On Mon, Apr 10, 2023 at 6:06 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>>>
>>>>>> On Mon, Apr 10, 2023 at 03:20:52PM +0800, Jason Wang wrote:
>>>>>>> On Mon, Apr 10, 2023 at 2:40 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>>>>>
>>>>>>>> On Mon, Apr 10, 2023 at 02:20:16PM +0800, Jason Wang wrote:
>>>>>>>>> On Mon, Apr 10, 2023 at 2:15 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On Mon, Apr 10, 2023 at 09:33:32AM +0800, Jason Wang wrote:
>>>>>>>>>>> This is fine for vDPA but not for virtio if the design can only work
>>>>>>>>>>> for some specific setups (OSes/archs).
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>
>>>>>>>>>> Well virtio legacy has a long history of documenting existing hacks :)
>>>>>>>>>
>>>>>>>>> Exactly, so the legacy behaviour is not (or can't be) defined by the
>>>>>>>>> spec but the codes.
>>>>>>>>
>>>>>>>> I mean driver behaviour derives from the code but we do document it in
>>>>>>>> the spec to help people build devices.
>>>>>>>>
>>>>>>>>
>>>>>>>>>> But yes, VIRTIO_F_ORDER_PLATFORM has to be documented.
>>>>>>>>>> And we have to decide what to do about ACCESS_PLATFORM since
>>>>>>>>>> there's a security problem if device allows not acking it.
>>>>>>>>>> Two options:
>>>>>>>>>> - relax the rules a bit and say device will assume ACCESS_PLATFORM
>>>>>>>>>> is acked anyway
>>>>>>>>>
>>>>>>>>> This will break legacy drivers which assume physical addresses.
>>>>>>>>
>>>>>>>> not that they are not already broken.
>>>>>>>
>>>>>>> I may miss something, the whole point is to allow legacy drivers to
>>>>>>> run otherwise a modern device is sufficient?
>>>>>>
>>>>>> yes and if legacy drivers don't work in a given setup then we
>>>>>> should not worry.
>>>>>>
>>>>>>>>
>>>>>>>>>> - a new flag that is insecure (so useful for sec but useless for dpdk) but optional
>>>>>>>>>
>>>>>>>>> This looks like a new "hack" for the legacy hacks.
>>>>>>>>
>>>>>>>> it's not just for legacy.
>>>>>>>
>>>>>>> We have the ACCESS_PLATFORM feature bit, what is the useage for this new flag?
>>>>>>
>>>>>>
>>>>>> ACCESS_PLATFORM is also a security boundary. so devices must fail
>>>>>> negotiation if it's not there. this new one won't be.
>>>>>>
>>>>>>
>>>>>>>>
>>>>>>>>> And what about ORDER_PLATFORM, I don't think we can modify legacy drivers...
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> You play some tricks with shadow VQ I guess.
>>>>>>>
>>>>>>> Do we really want to add a new feature in the virtio spec that can
>>>>>>> only work with the datapath mediation?
>>>>>>>
>>>>>>> Thanks
>>>>>>
>>>>>> As long as a feature is useful and can't be supported otherwise
>>>>>> we are out of options.
>>>>>
>>>>> Probably not? Is it as simple as relaxing this:
>>>>>
>>>>> "Transitional devices MUST expose the Legacy Interface in I/O space in BAR0."
>>>>>
>>>>> To allow memory space.
>>>>>
>>>>> This works for both software and hardware devices (I had a handy
>>>>> hardware that supports legacy virtio drivers in this way).
>>>>>
>>>>> Thanks
>>>>
>>>> Yes it is certainly simpler.
>>>>
>>>> Question: what happens if you try to run existing windows guests or dpdk
>>>> on these? Do they crash horribly or exit gracefully?
>>>
>>> Haven't tried DPDK and windows. But I remember DPDK supported legacy
>>> MMIO bars for a while.
>>
>> Regarding Windows drivers:
>> 1. We are acking VIRTIO_F_ACCESS_PLATFORM in the driver. But, if you
>> remember the "ATS" issue (Windows is either not detecting it or, even
>> if detected, not using it) - then actually, we are not forcing Windows
>> to remap the memory because Window fails to work with it correctly.
>> 2. Current Windows drivers implementation doesn't support MMIO bars.
>> We can enable the support if needed.
>>
>> Best regards,
>> Yan.
>
> The question was about old legacy drivers, not modern ones.
> What if they attach to a device and BAR0
> is a memory not an IO bar?
> Will they fail gracefully or crash?
The drivers should fail to load gracefully. There will be no crash, except in the case of the virtio-blk or virtio-scsi as a boot device.

>
>
>>
>>>
>>> Adding Maxime and Yan for comments here.
>>>
>>>>
>>>> The point of the capability is to allow using modern device ID so such
>>>> guests will not even try to bind.
>>>
>>> It means a mediation layer is required to use. Then it's not an issue
>>> for this simple relaxing any more?
>>>
>>> An advantage of such relaxing is that, for the legacy drivers like an
>>> ancient Linux version that can already do MMIO access to legacy BAR it
>>> can work without any mediation.
>>>
>>> At least, if ID can help, it can be used with this as well.
>>>
>>> Thanks
>>>
>>>
>>>
>>>>
>>>>
>>>>>> Keeping field practice things out of the
>>>>>> spec helps no one.
>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> MST
>>>>>>>>
>>>>>>
>>>>
>>>