Patchwork powerpc-powernv: added tce_get callback for powernv platform

login
register
mail settings
Submitter Alexey Kardashevskiy
Date Sept. 4, 2012, 7:35 a.m.
Message ID <1346744158-31190-1-git-send-email-aik@ozlabs.ru>
Download mbox | patch
Permalink /patch/181509/
State Not Applicable
Headers show

Comments

Alexey Kardashevskiy - Sept. 4, 2012, 7:35 a.m.
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(+)
Benjamin Herrenschmidt - Sept. 4, 2012, 7:41 p.m.
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);
>
David Gibson - Sept. 4, 2012, 10:35 p.m.
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.
Alexey Kardashevskiy - Sept. 5, 2012, 12:19 a.m.
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);
>>
>
>
Benjamin Herrenschmidt - Sept. 5, 2012, 12:32 a.m.
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.

Patch

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;
+}
+
 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);