linux-kernel - Re: [PATCH V4 4/6] mdev: introduce virtio device and its device ops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4a34d071-2e78-c37c-1046-7f9f6bb9e67f@redhat.com>
Date:   Mon, 21 Oct 2019 13:36:22 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Cornelia Huck <cohuck@...hat.com>
Cc:     kvm@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org,
        intel-gvt-dev@...ts.freedesktop.org, kwankhede@...dia.com,
        alex.williamson@...hat.com, mst@...hat.com, tiwei.bie@...el.com,
        virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
        maxime.coquelin@...hat.com, cunming.liang@...el.com,
        zhihong.wang@...el.com, rob.miller@...adcom.com,
        xiao.w.wang@...el.com, haotian.wang@...ive.com,
        zhenyuw@...ux.intel.com, zhi.a.wang@...el.com,
        jani.nikula@...ux.intel.com, joonas.lahtinen@...ux.intel.com,
        rodrigo.vivi@...el.com, airlied@...ux.ie, daniel@...ll.ch,
        farman@...ux.ibm.com, pasic@...ux.ibm.com, sebott@...ux.ibm.com,
        oberpar@...ux.ibm.com, heiko.carstens@...ibm.com,
        gor@...ux.ibm.com, borntraeger@...ibm.com, akrowiak@...ux.ibm.com,
        freude@...ux.ibm.com, lingshan.zhu@...el.com, idos@...lanox.com,
        eperezma@...hat.com, lulu@...hat.com, parav@...lanox.com,
        christophe.de.dinechin@...il.com, kevin.tian@...el.com,
        stefanha@...hat.com
Subject: Re: [PATCH V4 4/6] mdev: introduce virtio device and its device ops


On 2019/10/18 下午9:30, Cornelia Huck wrote:
> On Fri, 18 Oct 2019 18:55:02 +0800
> Jason Wang <jasowang@...hat.com> wrote:
>
>> On 2019/10/18 下午5:46, Cornelia Huck wrote:
>>> On Thu, 17 Oct 2019 18:48:34 +0800
>>> Jason Wang <jasowang@...hat.com> wrote:
>>>> + * @get_vendor_id:		Get virtio vendor id
>>>> + *				@mdev: mediated device
>>>> + *				Returns u32: virtio vendor id
>>> How is the vendor id defined? As for normal virtio-pci devices?
>>
>> The vendor that provides this device. So something like this
>>
>> I notice that MMIO also had this so it looks to me it's not pci specific.
> Ok. Would be good to specify this more explicitly.


Ok.


>
>>
>>>   
>>>> + * @get_status: 		Get the device status
>>>> + *				@mdev: mediated device
>>>> + *				Returns u8: virtio device status
>>>> + * @set_status: 		Set the device status
>>>> + *				@mdev: mediated device
>>>> + *				@status: virtio device status
>>>> + * @get_config: 		Read from device specific configuration space
>>>> + *				@mdev: mediated device
>>>> + *				@offset: offset from the beginning of
>>>> + *				configuration space
>>>> + *				@buf: buffer used to read to
>>>> + *				@len: the length to read from
>>>> + *				configration space
>>>> + * @set_config: 		Write to device specific configuration space
>>>> + *				@mdev: mediated device
>>>> + *				@offset: offset from the beginning of
>>>> + *				configuration space
>>>> + *				@buf: buffer used to write from
>>>> + *				@len: the length to write to
>>>> + *				configration space
>>>> + * @get_mdev_features:		Get the feature of virtio mdev device
>>>> + *				@mdev: mediated device
>>>> + *				Returns the mdev features (API) support by
>>>> + *				the device.
>>> What kind of 'features' are supposed to go in there? Are these bits,
>>> like you defined for VIRTIO_MDEV_F_VERSION_1 above?
>>
>> It's the API or mdev features other than virtio features. It could be
>> used by driver to determine the capability of the mdev device. Besides
>> _F_VERSION_1, we may add dirty page tracking etc which means we need new
>> device ops.
> Ok, so that's supposed to be distinct bits that can be or'ed together?


Yes.


> Makes sense, but probably needs some more documentation somewhere.


Let me add some doc above this helper.


>
>>
>>>   
>>>> + * @get_generation:		Get device generaton
>>>> + *				@mdev: mediated device
>>>> + *				Returns u32: device generation
>>> Is that callback mandatory?
>>
>> I think so, it's hard to emulate that completely in virtio-mdev transport.
> IIRC, the generation stuff is not mandatory in the current version of
> virtio, as not all transports have that concept.


It looks to me we can have workaround as what has been done by virtio.c. 
Return 0 if this helper is not provided.


>
> Generally, are any of the callbacks optional, or are all of them
> mandatory? From what I understand, you plan to add new things that
> depend on features... would that mean non-mandatory callbacks?


Yes, not all of them were mandatory, I will explicit explain which are 
mandatory and which are not.

Thanks