linux-kernel - Re: [PATCH v3 08/24] powerpc/spapr: vfio: Switch from iommu_table to new powerpc

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] [day] [month] [year] [list]

Date:	Thu, 05 Feb 2015 15:58:18 +1100
From:	Alexey Kardashevskiy <aik@...abs.ru>
To:	Alexander Graf <agraf@...e.de>,
	Alex Williamson <alex.williamson@...hat.com>
CC:	linuxppc-dev@...ts.ozlabs.org,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Michael Ellerman <mpe@...erman.id.au>,
	Gavin Shan <gwshan@...ux.vnet.ibm.com>,
	Alexander Gordeev <agordeev@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 08/24] powerpc/spapr: vfio: Switch from iommu_table
 to new powerpc_iommu

On 02/05/2015 12:32 AM, Alexander Graf wrote:
> 
> 
> On 03.02.15 01:12, Alex Williamson wrote:
>> On Thu, 2015-01-29 at 20:21 +1100, Alexey Kardashevskiy wrote:
>>> Modern IBM POWERPC systems support multiple (currently two) TCE tables
>>> per IOMMU group (a.k.a. PE). This adds a powerpc_iommu container
>>> for TCE tables. Right now just one table is supported.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@...abs.ru>
>>> ---
>>>  arch/powerpc/include/asm/iommu.h            |  18 ++--
>>>  arch/powerpc/kernel/eeh.c                   |   2 +-
>>>  arch/powerpc/kernel/iommu.c                 |  34 ++++----
>>>  arch/powerpc/platforms/powernv/pci-ioda.c   |  37 +++++---
>>>  arch/powerpc/platforms/powernv/pci-p5ioc2.c |  16 ++--
>>>  arch/powerpc/platforms/powernv/pci.c        |   2 +-
>>>  arch/powerpc/platforms/powernv/pci.h        |   4 +-
>>>  arch/powerpc/platforms/pseries/iommu.c      |   9 +-
>>>  drivers/vfio/vfio_iommu_spapr_tce.c         | 131 ++++++++++++++++++++--------
>>>  9 files changed, 170 insertions(+), 83 deletions(-)
>> [snip]
>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>>> index 29d5708..28909e1 100644
>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>>> @@ -84,7 +84,7 @@ static void decrement_locked_vm(long npages)
>>>   */
>>>  struct tce_container {
>>>  	struct mutex lock;
>>> -	struct iommu_table *tbl;
>>> +	struct iommu_group *grp;
>>>  	bool enabled;
>>>  };
>>>  
>>> @@ -104,16 +104,40 @@ static bool tce_check_page_size(struct page *page, unsigned page_shift)
>>>  	return false;
>>>  }
>>>  
>>> +static struct iommu_table *spapr_tce_find_table(
>>> +		struct tce_container *container,
>>> +		phys_addr_t ioba)
>>> +{
>>> +	long i;
>>> +	struct iommu_table *ret = NULL;
>>> +	struct powerpc_iommu *iommu = iommu_group_get_iommudata(container->grp);
>>> +
>>> +	mutex_lock(&container->lock);
>>> +	for (i = 0; i < POWERPC_IOMMU_MAX_TABLES; ++i) {
>>> +		struct iommu_table *tbl = &iommu->tables[i];
>>> +		unsigned long entry = ioba >> tbl->it_page_shift;
>>> +		unsigned long start = tbl->it_offset;
>>> +		unsigned long end = start + tbl->it_size;
>>> +
>>> +		if ((start <= entry) && (entry < end)) {
>>> +			ret = tbl;
>>> +			break;
>>> +		}
>>> +	}
>>> +	mutex_unlock(&container->lock);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>>  static int tce_iommu_enable(struct tce_container *container)
>>>  {
>>>  	int ret = 0;
>>> +	struct powerpc_iommu *iommu;
>>> +	struct iommu_table *tbl;
>>>  
>>> -	if (!container->tbl)
>>> +	if (!container->grp)
>>>  		return -ENXIO;
>>>  
>>> -	if (!current->mm)
>>> -		return -ESRCH; /* process exited */
>>> -
>>>  	if (container->enabled)
>>>  		return -EBUSY;
>>>  
>>> @@ -142,7 +166,12 @@ static int tce_iommu_enable(struct tce_container *container)
>>>  	 * as this information is only available from KVM and VFIO is
>>>  	 * KVM agnostic.
>>>  	 */
>>> -	ret = try_increment_locked_vm(IOMMU_TABLE_PAGES(container->tbl));
>>> +	iommu = iommu_group_get_iommudata(container->grp);
>>> +	if (!iommu)
>>> +		return -EFAULT;
>>> +
>>> +	tbl = &iommu->tables[0];
>>
>>
>> There should probably be a comment somewhere documenting that tables[0]
>> is the small window and presumably [1] will be the DDW.
> 
> Rather than a comment, how about an enum?


[0] could be DDW if the guest decides to remove the default window and
create one huge in its place - older guests (sles11sp3) did that (but they
could not cope with the huge window starting from zero), newer guests do
not try removing the default window but they might want to do this later.

So I am not so sure what kind of comment would be good here...



-- 
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/