powerpc-powernv: added tce_get callback for powernv platform

The upcoming VFIO support requires a way to know which
entry in the TCE map is not empty in order to do cleanup
at QEMU exit/crash. This patch adds such functionality
to POWERNV platform code.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/platforms/powernv/pci.c |    6 ++++++
 1 file changed, 6 insertions(+)

On Tue, 2012-09-04 at 17:35 +1000, Alexey Kardashevskiy wrote:
> The upcoming VFIO support requires a way to know which
> entry in the TCE map is not empty in order to do cleanup
> at QEMU exit/crash. This patch adds such functionality
> to POWERNV platform code.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  arch/powerpc/platforms/powernv/pci.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index be3cfc5..61f8068 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -447,6 +447,11 @@ static void pnv_tce_free(struct iommu_table *tbl, long index, long npages)
>  		pnv_tce_invalidate(tbl, tces, tcep - 1);
>  }
>  
> +static unsigned long pnv_tce_get(struct iommu_table *tbl, long index)
> +{
> +	return ((u64 *)tbl->it_base)[index - tbl->it_offset] & IOMMU_PAGE_MASK;
> +}

Why the masking here ?

Cheers,
Ben.

>  void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
>  			       void *tce_mem, u64 tce_size,
>  			       u64 dma_offset)
> @@ -597,6 +602,7 @@ void __init pnv_pci_init(void)
>  	ppc_md.pci_dma_dev_setup = pnv_pci_dma_dev_setup;
>  	ppc_md.tce_build = pnv_tce_build;
>  	ppc_md.tce_free = pnv_tce_free;
> +	ppc_md.tce_get = pnv_tce_get;
>  	ppc_md.pci_probe_mode = pnv_pci_probe_mode;
>  	set_pci_dma_ops(&dma_iommu_ops);
>

On Wed, Sep 05, 2012 at 05:41:42AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2012-09-04 at 17:35 +1000, Alexey Kardashevskiy wrote:
> > The upcoming VFIO support requires a way to know which
> > entry in the TCE map is not empty in order to do cleanup
> > at QEMU exit/crash. This patch adds such functionality
> > to POWERNV platform code.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >  arch/powerpc/platforms/powernv/pci.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> > index be3cfc5..61f8068 100644
> > --- a/arch/powerpc/platforms/powernv/pci.c
> > +++ b/arch/powerpc/platforms/powernv/pci.c
> > @@ -447,6 +447,11 @@ static void pnv_tce_free(struct iommu_table *tbl, long index, long npages)
> >  		pnv_tce_invalidate(tbl, tces, tcep - 1);
> >  }
> >  
> > +static unsigned long pnv_tce_get(struct iommu_table *tbl, long index)
> > +{
> > +	return ((u64 *)tbl->it_base)[index - tbl->it_offset] & IOMMU_PAGE_MASK;
> > +}
> 
> Why the masking here ?

Yes.  Especially since you're masking out the permission bits which
are actually the ones you want to determine if a TCE is empty or not.

On 05/09/12 05:41, Benjamin Herrenschmidt wrote:
> On Tue, 2012-09-04 at 17:35 +1000, Alexey Kardashevskiy wrote:
>> The upcoming VFIO support requires a way to know which
>> entry in the TCE map is not empty in order to do cleanup
>> at QEMU exit/crash. This patch adds such functionality
>> to POWERNV platform code.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   arch/powerpc/platforms/powernv/pci.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index be3cfc5..61f8068 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -447,6 +447,11 @@ static void pnv_tce_free(struct iommu_table *tbl, long index, long npages)
>>   		pnv_tce_invalidate(tbl, tces, tcep - 1);
>>   }
>>
>> +static unsigned long pnv_tce_get(struct iommu_table *tbl, long index)
>> +{
>> +	return ((u64 *)tbl->it_base)[index - tbl->it_offset] & IOMMU_PAGE_MASK;
>> +}
>
> Why the masking here ?


Oops. No reason. Will remove.


>
> Cheers,
> Ben.
>
>>   void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
>>   			       void *tce_mem, u64 tce_size,
>>   			       u64 dma_offset)
>> @@ -597,6 +602,7 @@ void __init pnv_pci_init(void)
>>   	ppc_md.pci_dma_dev_setup = pnv_pci_dma_dev_setup;
>>   	ppc_md.tce_build = pnv_tce_build;
>>   	ppc_md.tce_free = pnv_tce_free;
>> +	ppc_md.tce_get = pnv_tce_get;
>>   	ppc_md.pci_probe_mode = pnv_pci_probe_mode;
>>   	set_pci_dma_ops(&dma_iommu_ops);
>>
>
>

On Wed, 2012-09-05 at 10:19 +1000, Alexey Kardashevskiy wrote:
> >> +static unsigned long pnv_tce_get(struct iommu_table *tbl, long
> index)
> >> +{
> >> +    return ((u64 *)tbl->it_base)[index - tbl->it_offset] &
> IOMMU_PAGE_MASK;
> >> +}
> >
> > Why the masking here ?
> 
> 
> Oops. No reason. Will remove.

Right. The caller wants to know both whether the low bits are set and
whether there's an address up.

On the H_PUT_TCE path, you want to make sure:

 - If any of the low bit is set, set the TCE entry & get_page()
 - If none, then clear the whole entry (ignore the high bits passed by
the guest) and maybe put_page() the old page

IE the TCE either contains a valid page address + low bit(s) or all 0

That way, on the cleanup path, you can check the low bits only to decide
whether to cleanup, and if any is set, you know both your direction
(writeable vs. read only) and whether something was there at all.

You do not want to ever compare the high bits (address) to 0. While we
never do it in practice I suspect, there's no fundamental reason why a
physical address of 0 is incorrect in a TCE.

Cheers,
Ben.