From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: Pierre Morel <pmorel@linux.vnet.ibm.com>,
linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org, freude@de.ibm.com, schwidefsky@de.ibm.com,
heiko.carstens@de.ibm.com, borntraeger@de.ibm.com,
kwankhede@nvidia.com, bjsdjshi@linux.vnet.ibm.com,
pbonzini@redhat.com, alex.williamson@redhat.com,
alifm@linux.vnet.ibm.com, mjrosato@linux.vnet.ibm.com,
jjherne@linux.vnet.ibm.com, thuth@redhat.com,
pasic@linux.vnet.ibm.com, fiuczy@linux.vnet.ibm.com,
buendgen@de.ibm.com
Subject: Re: [PATCH v2 05/15] s390: vfio-ap: base implementation of VFIO AP device driver
Date: Wed, 28 Feb 2018 15:34:32 -0500 [thread overview]
Message-ID: <6ec02d29-a0fb-a60e-0147-8c7c7530c3ae@linux.vnet.ibm.com> (raw)
In-Reply-To: <20180228191019.20507888.cohuck@redhat.com>
On 02/28/2018 01:10 PM, Cornelia Huck wrote:
> On Wed, 28 Feb 2018 11:43:37 -0500
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>
>> On 02/28/2018 10:33 AM, Pierre Morel wrote:
>>> On 27/02/2018 15:28, Tony Krowiak wrote:
> (...)
>
>>>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>>>> index cbe1d97..9f23caf 100644
>>>> --- a/arch/s390/Kconfig
>>>> +++ b/arch/s390/Kconfig
>>>> @@ -771,6 +771,14 @@ config VFIO_CCW
>>>> To compile this driver as a module, choose M here: the
>>>> module will be called vfio_ccw.
>>>>
>>>> +config VFIO_AP
>>>> + def_tristate m
> Any reason you default to m instead of n here?
None in particular, is there a good reason to change this to n?
>
>>>> + prompt "Support for virtual Adjunct Processor device interface"
>>> The VFIO AP devices are not virtual.
>>> What about
>>> "VFIO support for AP devices"
>> Sounds good.
> +1
>
>>>
>>>> + depends on ZCRYPT && VFIO_MDEV_DEVICE
>>>> + help
>>>> + driver grants access to Adjunct Processor (AP) devices
> s/driver/This driver/
Okay, I'll make the change
>
>>>> + via the VFIO mediated device interface.
> You also might want to add
>
> "To compile this driver as a module, choose M here: the
> module will be called..."
Will do
>
>>>> +
>>>> endmenu
> It's a tad confusing to find this in the I/O submenu, but I don't
> really have a better idea.
I wasn't sure either, but couldn't think of a better location. Anybody
else have any ideas?
>
>>>> menu "Dump support"
>>>> diff --git a/arch/s390/configs/default_defconfig
>>>> b/arch/s390/configs/default_defconfig
>>>> index 5af8458..40fa3f6 100644
>>>> --- a/arch/s390/configs/default_defconfig
>>>> +++ b/arch/s390/configs/default_defconfig
>>> Not sure that this file belongs to this patch
>> Neither am I, but at the time I inserted this here - well before August
>> of last year - I was using vfio-ccw as a model.
>> If someone can verify this does not belong here, I'd be more than happy
>> to remove it.
> I don't see any entry for VFIO_CCW in there?
There isn't now, but there was at the time I first started working on this.
My first pass was a lot of cut-and-paste followed by modification of the
vfio_ccw implementation, but that was long ago. This is a remnant of that
time. Like I said, I'd be happy to remove this.
>
>>>
>>>> @@ -719,3 +719,6 @@ CONFIG_APPLDATA_BASE=y
>>>> CONFIG_KVM=m
>>>> CONFIG_KVM_S390_UCONTROL=y
>>>> CONFIG_VHOST_NET=m
>>>> +VFIO_MDEV=m
>>>> +VFIO_MDEV_DEVICE=m
>>>> +CONFIG_VFIO_AP=m
>>> What is your goal when modifying this three files?
>>> Could you add a comment in the commit message?
>> As stated above, this was originally based on the vfio-ccw model and has
>> been in the
>> patch series since its inception. I'd be happy to remove it if it is not
>> necessary.
> I'd vote for removing it.
Consider them gone.
>
> (...)
>
>>>> +static int vfio_ap_matrix_dev_create(void)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME);
>>>> +
>>>> + ret = PTR_ERR_OR_ZERO(vfio_ap_root_device);
>>> IS_ERR() is enough, root_device_register() never return NULL.
>> I searched the kernel code to look at other places the
>> root_device_register()
>> function is called to see how the return value is handled. I've seen all
>> of the
>> following used:
>> if (IS_ERR())
>> ret = PTR_ERR()
>> PTR_ERR()
>> PTR_ERR_OR_ZERO()
>>
>> I'm not sure why this is a concern, but I'll use the first option above
>> since PTR_ERR_OR_ZERO() also embeds the first option.
> PTR_ERR_OR_ZERO() seems like the best choice for the way the return
> code is processed here. (It's just unfortunate that its name conjures
> up connotations of NULL-pointer handling.)
I changed it to:
ret = IS_ERR(vfio_ap_root_device);
if (ret) {
ret = PTR_ERR(vfio_ap_root_device);
goto done;
}
Hopefully everybody is happy with this.
>
>>>> + if (ret)
>>>> + goto done;
next prev parent reply other threads:[~2018-02-28 20:34 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-27 14:27 [PATCH v2 00/15] s390: vfio-ap: guest dedicated crypto adapters Tony Krowiak
2018-02-27 14:27 ` [PATCH v2 01/15] KVM: s390: refactor crypto initialization Tony Krowiak
2018-02-28 17:37 ` Cornelia Huck
2018-02-28 21:23 ` Tony Krowiak
2018-03-01 9:59 ` Cornelia Huck
2018-03-14 16:02 ` Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 02/15] s390: vsie: implement AP support for second level guest Tony Krowiak
2018-02-28 9:39 ` David Hildenbrand
2018-02-28 10:03 ` Pierre Morel
2018-02-27 14:28 ` [PATCH v2 03/15] s390: zcrypt: externalize AP instructions available function Tony Krowiak
2018-02-28 11:40 ` Harald Freudenberger
2018-02-28 17:41 ` Cornelia Huck
2018-03-01 8:38 ` Harald Freudenberger
2018-02-27 14:28 ` [PATCH v2 04/15] KVM: s390: CPU model support for AP virtualization Tony Krowiak
2018-02-28 9:48 ` David Hildenbrand
2018-02-28 11:40 ` Christian Borntraeger
2018-02-28 11:58 ` David Hildenbrand
2018-02-28 15:59 ` Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 05/15] s390: vfio-ap: base implementation of VFIO AP device driver Tony Krowiak
2018-02-28 15:33 ` Pierre Morel
2018-02-28 16:43 ` Tony Krowiak
2018-02-28 18:10 ` Cornelia Huck
2018-02-28 20:34 ` Tony Krowiak [this message]
2018-02-27 14:28 ` [PATCH v2 06/15] s390: vfio-ap: register matrix device with VFIO mdev framework Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 07/15] KVM: s390: Interfaces to configure/deconfigure guest's AP matrix Tony Krowiak
2018-02-28 16:15 ` Pierre Morel
2018-02-28 19:11 ` Tony Krowiak
2018-02-28 18:50 ` Tony Krowiak
2018-02-28 19:38 ` Tony Krowiak
2018-03-08 23:03 ` Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 08/15] KVM: s390: interface to enable AP execution mode Tony Krowiak
2018-02-28 9:42 ` David Hildenbrand
2018-02-28 20:37 ` Tony Krowiak
2018-03-01 9:32 ` David Hildenbrand
2018-02-28 9:44 ` David Hildenbrand
2018-02-28 20:39 ` Tony Krowiak
2018-03-01 9:35 ` David Hildenbrand
2018-03-02 19:54 ` Tony Krowiak
2018-03-14 16:29 ` Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 09/15] s390: vfio-ap: sysfs interfaces to configure adapters Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 10/15] s390: vfio-ap: sysfs interfaces to configure domains Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 11/15] s390: vfio-ap: sysfs interfaces to configure control domains Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 12/15] s390: vfio-ap: sysfs interface to view matrix mdev matrix Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 13/15] KVM: s390: Configure the guest's CRYCB Tony Krowiak
2018-02-28 9:49 ` David Hildenbrand
2018-02-28 20:45 ` Tony Krowiak
2018-03-01 9:37 ` David Hildenbrand
2018-03-01 20:42 ` Tony Krowiak
2018-03-02 10:08 ` David Hildenbrand
2018-03-02 19:48 ` Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 14/15] s390: vfio-ap: implement VFIO_DEVICE_GET_INFO ioctl Tony Krowiak
2018-02-27 14:28 ` [PATCH v2 15/15] s390: doc: detailed specifications for AP virtualization Tony Krowiak
2018-02-27 15:57 ` Cornelia Huck
2018-02-27 18:12 ` Tony Krowiak
2018-02-27 14:58 ` [PATCH v2 00/15] s390: vfio-ap: guest dedicated crypto adapters Cornelia Huck
2018-02-27 15:57 ` Tony Krowiak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6ec02d29-a0fb-a60e-0147-8c7c7530c3ae@linux.vnet.ibm.com \
--to=akrowiak@linux.vnet.ibm.com \
--cc=alex.williamson@redhat.com \
--cc=alifm@linux.vnet.ibm.com \
--cc=bjsdjshi@linux.vnet.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=buendgen@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=fiuczy@linux.vnet.ibm.com \
--cc=freude@de.ibm.com \
--cc=heiko.carstens@de.ibm.com \
--cc=jjherne@linux.vnet.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mjrosato@linux.vnet.ibm.com \
--cc=pasic@linux.vnet.ibm.com \
--cc=pbonzini@redhat.com \
--cc=pmorel@linux.vnet.ibm.com \
--cc=schwidefsky@de.ibm.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).