linux-kernel - Re: [PATCH 1/2] vfio, platform: add support for ACPI while detecting the reset driver

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: <20160311040537.4067a210@arm.com>
Date:	Fri, 11 Mar 2016 04:05:37 +0000
From:	Marc Zyngier <marc.zyngier@....com>
To:	Sinan Kaya <okaya@...eaurora.org>
Cc:	Eric Auger <eric.auger@...aro.org>, <kvm@...r.kernel.org>,
	<timur@...eaurora.org>, <cov@...eaurora.org>, <jcm@...hat.com>,
	<shankerd@...eaurora.org>, <vikrams@...eaurora.org>,
	<mark.rutland@....com>, <devicetree@...r.kernel.org>,
	<vinod.koul@...el.com>, <agross@...eaurora.org>,
	<linux-arm-msm@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	Baptiste Reynal <b.reynal@...tualopensystems.com>,
	Alex Williamson <alex.williamson@...hat.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	"Arnd Bergmann" <arnd@...db.de>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] vfio, platform: add support for ACPI while
 detecting the reset driver

On Thu, 10 Mar 2016 12:13:28 -0500
Sinan Kaya <okaya@...eaurora.org> wrote:

> On 3/10/2016 5:11 AM, Eric Auger wrote:
> > Hi Sinan,
> > On 03/08/2016 04:33 PM, Sinan Kaya wrote:
> >> The  code is using the compatible DT string to associate a reset driver with
> >> the actual device itself. The compatible string does not exist on ACPI
> >> based systems. HID is the unique identifier for a device driver instead.
> >> The change allows a driver to register with DT compatible string or ACPI
> >> HID and then match the object with one of these conditions.
> >>
> >> Rules for loading the reset driver are as follow:
> >> - ACPI HID needs match for ACPI systems
> >> - DT compat needs to match for OF systems
> >>
> >> Tested-by: Eric Auger <eric.auger@...aro.org> (device tree only)
> >> Tested-by: Shanker Donthineni <shankerd@...eaurora.org> (ACPI only)
> >> Signed-off-by: Sinan Kaya <okaya@...eaurora.org>
> >> ---
> >>  .../vfio/platform/reset/vfio_platform_amdxgbe.c    |   4 +-
> >>  .../platform/reset/vfio_platform_calxedaxgmac.c    |   4 +-
> >>  drivers/vfio/platform/vfio_platform_common.c       | 112 +++++++++++++++++----
> >>  drivers/vfio/platform/vfio_platform_private.h      |  43 ++++----
> >>  4 files changed, 125 insertions(+), 38 deletions(-)

[...]

> >> -	vfio_platform_get_reset(vdev);
> >> +	ret = vfio_platform_get_reset(vdev);
> >> +	if (ret) {
> >> +		iommu_group_put(group);
> >> +		return ret;
> >> +	}
> > This change is not related to your commit message. Also here you change
> > the use model of VFIO platform and forbid any usage if no reset module
> > is available, right? I don't think this is something we discussed and I
> > think it removes some flexibility. Currently a warning is emitted in
> > case we don't have a reset function.
> 
> Well, I haven't seen that warning during testing. I was trying to be more 
> proactive.
> 
> I'm fine removing these checks but not having a reset driver needs a really
> big warning here.

This code really needs to be moved to a separate patch, but I agree
overall: a reset function should be made mandatory - I cannot imagine
trusting the HW to be left in a sane state after being used by a guest.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.