linux-kernel - Re: [PATCH kernel v10 20/34] powerpc/powernv/ioda2: Move TCE kill register address to PE

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: <555418FC.1070906@ozlabs.ru>
Date:	Thu, 14 May 2015 13:39:40 +1000
From:	Alexey Kardashevskiy <aik@...abs.ru>
To:	Gavin Shan <gwshan@...ux.vnet.ibm.com>
CC:	linuxppc-dev@...ts.ozlabs.org,
	David Gibson <david@...son.dropbear.id.au>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Alex Williamson <alex.williamson@...hat.com>,
	Wei Yang <weiyang@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH kernel v10 20/34] powerpc/powernv/ioda2: Move TCE kill
 register address to PE

On 05/14/2015 12:10 PM, Gavin Shan wrote:
> On Tue, May 12, 2015 at 01:39:09AM +1000, Alexey Kardashevskiy wrote:
>> At the moment the DMA setup code looks for the "ibm,opal-tce-kill" property
>> which contains the TCE kill register address. Writes to this register
>> invalidates TCE cache on IODA/IODA2 hub.
>>
>> This moves the register address from iommu_table to pnv_ioda_pe as:
>> 1) When we get 2 tables per PE, this register will be used for both tables;
>> 2) When we get TCE tables sharing, we will need to invalidate every
>> IOMMU group (i.e. PE) which is using this table and each PE has
>> its own invalidate register.
>>
>
> Actually, it's the virtual address of IO remapped PHB hardware register.
> So it would be a property of PHB (struct pnv_phb). As the PE is connecting
> with IOMMU table group. The virtual address can be retrieved by the path:
> iommu_table -> iommu_table_group -> pnv_ioda_pe -> pnv_phb. However, I
> don't insist and you have the best judge on it :-)

Are you suggesting moving pe->tce_inval_reg from pnv_ioda_pe to pnv_phb?

>
>> This moves the property reading/remapping code to a helper to reduce
>> code duplication. Although this change is not required for IODA1, this
>> changes it as well to reduce code duplication.
>>
>> This adds a new pnv_pci_ioda2_tvt_invalidate() helper which invalidates
>> the entire table. It should be called after every call to
>> opal_pci_map_pe_dma_window(). It was not required before because
>> there is just a single TCE table and 64bit DMA is handled via bypass
>> window (which has no table so no chache is used) but this is going
>> to change with Dynamic DMA windows (DDW).
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@...abs.ru>
>
> Reviewed-by: Gavin Shan <gwshan@...ux.vnet.ibm.com>
>
> Thanks,
> Gavin
>
>> ---
>> Changes:
>> v10:
>> * fixed error from checkpatch.pl
>> * removed comment at "ibm,opal-tce-kill" parsing as irrelevant
>> * s/addr/val/ in pnv_pci_ioda2_tvt_invalidate() as it was not a kernel address
>>
>> v9:
>> * new in the series
>> ---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 64 ++++++++++++++++++-------------
>> arch/powerpc/platforms/powernv/pci.h      |  1 +
>> 2 files changed, 39 insertions(+), 26 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index 35ab19c8..f972e40 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -1680,7 +1680,7 @@ static void pnv_pci_ioda1_tce_invalidate(struct iommu_table *tbl,
>> 			struct pnv_ioda_pe, table_group);
>> 	__be64 __iomem *invalidate = rm ?
>> 		(__be64 __iomem *)pe->tce_inval_reg_phys :
>> -		(__be64 __iomem *)tbl->it_index;
>> +		pe->tce_inval_reg;
>> 	unsigned long start, end, inc;
>> 	const unsigned shift = tbl->it_page_shift;
>>
>> @@ -1751,6 +1751,18 @@ static struct iommu_table_ops pnv_ioda1_iommu_ops = {
>> 	.get = pnv_tce_get,
>> };
>>
>> +static inline void pnv_pci_ioda2_tvt_invalidate(struct pnv_ioda_pe *pe)
>> +{
>> +	/* 01xb - invalidate TCEs that match the specified PE# */
>> +	unsigned long val = (0x4ull << 60) | (pe->pe_number & 0xFF);
>> +
>> +	if (!pe->tce_inval_reg)
>> +		return;
>> +
>> +	mb(); /* Ensure above stores are visible */
>> +	__raw_writeq(cpu_to_be64(val), pe->tce_inval_reg);
>> +}
>> +
>
> The function name sounds it's to invalidate TVE cache. Actually, it's invalidting
> TCE cache. So I guess the function name pnv_pci_ioda2_tce_invalidate() would be
> more accurate.

TVT vs. TVE distinction is not clear for me, sorry :)

There is a function with exactly the same name as you proposed, below in 
this mail, it invalidates cache.
May be s/pnv_pci_ioda2_tvt_invalidate/pnv_pci_ioda2_invalidate_entire_cache/ ?




-- 
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/