Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Alexey Kardashevskiy



On 23/06/2020 12:43, Leonardo Bras wrote:
> On Tue, 2020-06-23 at 12:35 +1000, Alexey Kardashevskiy wrote:
>>> I am not sure if this is true in general, but in this device (SR-IOV
>>> VF) I am testing it will return 0 windows if the default DMA window is
>>> not deleted, and 1 after it's deleted.
>>
>> Since pHyp can only create windows in "64bit space", now (I did not know
>> until a few month back) I expect that thing to return "1" always no
>> matter what happened to the default window. And removing the default
>> window will only affect the maximum number of TCEs but not the number of
>> possible windows.
> 
> Humm, something gone wrong then.
> 
> This patchset was developed mostly because on testing, DDW would never
> be created because windows_available would always be 0.

? On phyp, if there is a huge window, then it can be 0 or 1. On KVM, it
is 0 or 1 or 2.


> 
> I will take a deeper look in that.
> 
> Best regards,
> Leonardo
> 

-- 
Alexey


Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Leonardo Bras
On Tue, 2020-06-23 at 12:35 +1000, Alexey Kardashevskiy wrote:
> > I am not sure if this is true in general, but in this device (SR-IOV
> > VF) I am testing it will return 0 windows if the default DMA window is
> > not deleted, and 1 after it's deleted.
> 
> Since pHyp can only create windows in "64bit space", now (I did not know
> until a few month back) I expect that thing to return "1" always no
> matter what happened to the default window. And removing the default
> window will only affect the maximum number of TCEs but not the number of
> possible windows.

Humm, something gone wrong then.

This patchset was developed mostly because on testing, DDW would never
be created because windows_available would always be 0.

I will take a deeper look in that.

Best regards,
Leonardo



Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Alexey Kardashevskiy



On 23/06/2020 12:31, Leonardo Bras wrote:
> On Tue, 2020-06-23 at 11:11 +1000, Alexey Kardashevskiy wrote:
>>
>> On 23/06/2020 04:59, Leonardo Bras wrote:
>>> Hello Alexey, thanks for the feedback!
>>>
>>> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
 On 19/06/2020 15:06, Leonardo Bras wrote:
> On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
> default DMA window for the device, before attempting to configure a DDW,
> in order to make the maximum resources available for the next DDW to be
> created.
>
> This is a requirement for some devices to use DDW, given they only
> allow one DMA window.
>
> If setting up a new DDW fails anywhere after the removal of this
> default DMA window, restore it using reset_dma_window.

 Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
 as pHyp can only create a single window and it has to map at
 0x800.... They probably do not care though.

 Under KVM, this will fail as VFIO allows creating  2 windows and it
 starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
 the window address == 0 as a failure. And we want to keep both DMA
 windows for PCI adapters with both 64bit and 32bit PCI functions (I
 heard AMD GPU video + audio are like this) or someone could hotplug
 32bit DMA device on a vphb with already present 64bit DMA window so we
 do not remove the default window.
>>>
>>> Well, then I would suggest doing something like this:
>>> query_ddw(...);
>>> if (query.windows_available == 0){
>>> remove_dma_window(...,default_win);
>>> query_ddw(...);
>>> }
>>>
>>> This would make sure to cover cases of windows available == 1
>>> and windows available > 1; 
>>
>> Is "1" what pHyp returns on query? And was it always like that? Then it
>> is probably ok. I just never really explored the idea of removing the
>> default window as we did not have to.
> 
> I am not sure if this is true in general, but in this device (SR-IOV
> VF) I am testing it will return 0 windows if the default DMA window is
> not deleted, and 1 after it's deleted.


Since pHyp can only create windows in "64bit space", now (I did not know
until a few month back) I expect that thing to return "1" always no
matter what happened to the default window. And removing the default
window will only affect the maximum number of TCEs but not the number of
possible windows.


-- 
Alexey


Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Leonardo Bras
On Tue, 2020-06-23 at 11:11 +1000, Alexey Kardashevskiy wrote:
> 
> On 23/06/2020 04:59, Leonardo Bras wrote:
> > Hello Alexey, thanks for the feedback!
> > 
> > On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
> > > On 19/06/2020 15:06, Leonardo Bras wrote:
> > > > On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
> > > > default DMA window for the device, before attempting to configure a DDW,
> > > > in order to make the maximum resources available for the next DDW to be
> > > > created.
> > > > 
> > > > This is a requirement for some devices to use DDW, given they only
> > > > allow one DMA window.
> > > > 
> > > > If setting up a new DDW fails anywhere after the removal of this
> > > > default DMA window, restore it using reset_dma_window.
> > > 
> > > Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
> > > as pHyp can only create a single window and it has to map at
> > > 0x800.... They probably do not care though.
> > > 
> > > Under KVM, this will fail as VFIO allows creating  2 windows and it
> > > starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
> > > the window address == 0 as a failure. And we want to keep both DMA
> > > windows for PCI adapters with both 64bit and 32bit PCI functions (I
> > > heard AMD GPU video + audio are like this) or someone could hotplug
> > > 32bit DMA device on a vphb with already present 64bit DMA window so we
> > > do not remove the default window.
> > 
> > Well, then I would suggest doing something like this:
> > query_ddw(...);
> > if (query.windows_available == 0){
> > remove_dma_window(...,default_win);
> > query_ddw(...);
> > }
> > 
> > This would make sure to cover cases of windows available == 1
> > and windows available > 1; 
> 
> Is "1" what pHyp returns on query? And was it always like that? Then it
> is probably ok. I just never really explored the idea of removing the
> default window as we did not have to.

I am not sure if this is true in general, but in this device (SR-IOV
VF) I am testing it will return 0 windows if the default DMA window is
not deleted, and 1 after it's deleted.

> 
> 
> > > The last discussed thing I remember was that there was supposed to be a
> > > new bit in "ibm,architecture-vec-5" (forgot the details), we could use
> > > that to decide whether to keep the default window or not, like this.
> > 
> > I checked on the latest LoPAR draft (soon to be published), for the
> > ibm,architecture-vec 'option array 5' and this entry was the only
> > recently added one that is related to this patchset:
> > 
> > Byte 8 - Bit 0:
> > SRIOV Virtual Functions Support Dynamic DMA Windows (DDW):
> > 0: SRIOV Virtual Functions do not support DDW
> > 1: SRIOV Virtual Functions do support DDW
> > 
> > Isn't this equivalent to having a "ibm,ddw-applicable" property?
> 
> I am not sure, is there anything else to this new bit?

I copied everything from the LoPAR, and IIRC the ACR for this change
only described this change in the document.


>  I'd think if the
> client supports it, then pHyp will create one 64bit window per a PE and
> DDW API won't be needed. Thanks,

That would make sense, and be great.
I will dig some more.

Thank you!

> > > > Signed-off-by: Leonardo Bras 
> > > > ---
> > > >  arch/powerpc/platforms/pseries/iommu.c | 20 +---
> > > >  1 file changed, 17 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c 
> > > > b/arch/powerpc/platforms/pseries/iommu.c
> > > > index de633f6ae093..68d1ea957ac7 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> > > > device_node *pdn)
> > > > u64 dma_addr, max_addr;
> > > > struct device_node *dn;
> > > > u32 ddw_avail[3];
> > > > +
> > > > struct direct_window *window;
> > > > -   struct property *win64;
> > > > +   struct property *win64, *dfl_win;
> > > 
> > > Make it "default_win" or "def_win", "dfl" hurts to read :)
> > 
> > Sure, no problem :)
> > 
> > > > struct dynamic_dma_window_prop *ddwprop;
> > > > struct failed_ddw_pdn *fpdn;
> > > >  
> > > > @@ -1110,8 +,19 @@ static u64 enable_ddw(struct pci_dev *dev, 
> > > > struct device_node *pdn)
> > > > if (ret)
> > > > goto out_failed;
> > > >  
> > > > -   /*
> > > > -* Query if there is a second window of size to map the
> > > > +   /*
> > > > +* First step of setting up DDW is removing the default DMA 
> > > > window,
> > > > +* if it's present. It will make all the resources available to 
> > > > the
> > > > +* new DDW window.
> > > > +* If anything fails after this, we need to restore it.
> > > > +*/
> > > > +
> > > > +   dfl_win = of_find_property(pdn, 

Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Alexey Kardashevskiy



On 23/06/2020 04:59, Leonardo Bras wrote:
> Hello Alexey, thanks for the feedback!
> 
> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>>
>> On 19/06/2020 15:06, Leonardo Bras wrote:
>>> On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
>>> default DMA window for the device, before attempting to configure a DDW,
>>> in order to make the maximum resources available for the next DDW to be
>>> created.
>>>
>>> This is a requirement for some devices to use DDW, given they only
>>> allow one DMA window.
>>>
>>> If setting up a new DDW fails anywhere after the removal of this
>>> default DMA window, restore it using reset_dma_window.
>>
>> Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
>> as pHyp can only create a single window and it has to map at
>> 0x800.... They probably do not care though.
>>
>> Under KVM, this will fail as VFIO allows creating  2 windows and it
>> starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
>> the window address == 0 as a failure. And we want to keep both DMA
>> windows for PCI adapters with both 64bit and 32bit PCI functions (I
>> heard AMD GPU video + audio are like this) or someone could hotplug
>> 32bit DMA device on a vphb with already present 64bit DMA window so we
>> do not remove the default window.
> 
> Well, then I would suggest doing something like this:
>   query_ddw(...);
>   if (query.windows_available == 0){
>   remove_dma_window(...,default_win);
>   query_ddw(...);
>   }
> 
> This would make sure to cover cases of windows available == 1
> and windows available > 1; 


Is "1" what pHyp returns on query? And was it always like that? Then it
is probably ok. I just never really explored the idea of removing the
default window as we did not have to.


>> The last discussed thing I remember was that there was supposed to be a
>> new bit in "ibm,architecture-vec-5" (forgot the details), we could use
>> that to decide whether to keep the default window or not, like this.
> 
> I checked on the latest LoPAR draft (soon to be published), for the
> ibm,architecture-vec 'option array 5' and this entry was the only
> recently added one that is related to this patchset:
> 
> Byte 8 - Bit 0:
> SRIOV Virtual Functions Support Dynamic DMA Windows (DDW):
> 0: SRIOV Virtual Functions do not support DDW
> 1: SRIOV Virtual Functions do support DDW
> 
> Isn't this equivalent to having a "ibm,ddw-applicable" property?

I am not sure, is there anything else to this new bit? I'd think if the
client supports it, then pHyp will create one 64bit window per a PE and
DDW API won't be needed. Thanks,


> 
> 
>>
>>> Signed-off-by: Leonardo Bras 
>>> ---
>>>  arch/powerpc/platforms/pseries/iommu.c | 20 +---
>>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c 
>>> b/arch/powerpc/platforms/pseries/iommu.c
>>> index de633f6ae093..68d1ea957ac7 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
>>> device_node *pdn)
>>> u64 dma_addr, max_addr;
>>> struct device_node *dn;
>>> u32 ddw_avail[3];
>>> +
>>> struct direct_window *window;
>>> -   struct property *win64;
>>> +   struct property *win64, *dfl_win;
>>
>> Make it "default_win" or "def_win", "dfl" hurts to read :)
> 
> Sure, no problem :)
> 
>>
>>> struct dynamic_dma_window_prop *ddwprop;
>>> struct failed_ddw_pdn *fpdn;
>>>  
>>> @@ -1110,8 +,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
>>> device_node *pdn)
>>> if (ret)
>>> goto out_failed;
>>>  
>>> -   /*
>>> -* Query if there is a second window of size to map the
>>> +   /*
>>> +* First step of setting up DDW is removing the default DMA window,
>>> +* if it's present. It will make all the resources available to the
>>> +* new DDW window.
>>> +* If anything fails after this, we need to restore it.
>>> +*/
>>> +
>>> +   dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
>>> +   if (dfl_win)
>>> +   remove_dma_window(pdn, ddw_avail, dfl_win);
>>
>> Before doing so, you want to make sure that the "reset" is actually
>> supported. Thanks,
> 
> Good catch, I will improve that.
> 
>>
>>
>>> +
>>> +   /*
>>> +* Query if there is a window of size to map the
>>>  * whole partition.  Query returns number of windows, largest
>>>  * block assigned to PE (partition endpoint), and two bitmasks
>>>  * of page sizes: supported and supported for migrate-dma.
>>> @@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
>>> device_node *pdn)
>>> kfree(win64);
>>>  
>>>  out_failed:
>>> +   if (dfl_win)
>>> +   reset_dma_window(dev, pdn);
>>>  
>>> fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
>>> if (!fpdn)
>>>
> 
> Best regards,

Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Leonardo Bras
Hello Alexey, thanks for the feedback!

On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
> 
> On 19/06/2020 15:06, Leonardo Bras wrote:
> > On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
> > default DMA window for the device, before attempting to configure a DDW,
> > in order to make the maximum resources available for the next DDW to be
> > created.
> > 
> > This is a requirement for some devices to use DDW, given they only
> > allow one DMA window.
> > 
> > If setting up a new DDW fails anywhere after the removal of this
> > default DMA window, restore it using reset_dma_window.
> 
> Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
> as pHyp can only create a single window and it has to map at
> 0x800.... They probably do not care though.
> 
> Under KVM, this will fail as VFIO allows creating  2 windows and it
> starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
> the window address == 0 as a failure. And we want to keep both DMA
> windows for PCI adapters with both 64bit and 32bit PCI functions (I
> heard AMD GPU video + audio are like this) or someone could hotplug
> 32bit DMA device on a vphb with already present 64bit DMA window so we
> do not remove the default window.

Well, then I would suggest doing something like this:
query_ddw(...);
if (query.windows_available == 0){
remove_dma_window(...,default_win);
query_ddw(...);
}

This would make sure to cover cases of windows available == 1
and windows available > 1; 

> The last discussed thing I remember was that there was supposed to be a
> new bit in "ibm,architecture-vec-5" (forgot the details), we could use
> that to decide whether to keep the default window or not, like this.

I checked on the latest LoPAR draft (soon to be published), for the
ibm,architecture-vec 'option array 5' and this entry was the only
recently added one that is related to this patchset:

Byte 8 - Bit 0:
SRIOV Virtual Functions Support Dynamic DMA Windows (DDW):
0: SRIOV Virtual Functions do not support DDW
1: SRIOV Virtual Functions do support DDW

Isn't this equivalent to having a "ibm,ddw-applicable" property?


> 
> > Signed-off-by: Leonardo Bras 
> > ---
> >  arch/powerpc/platforms/pseries/iommu.c | 20 +---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c 
> > b/arch/powerpc/platforms/pseries/iommu.c
> > index de633f6ae093..68d1ea957ac7 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> > device_node *pdn)
> > u64 dma_addr, max_addr;
> > struct device_node *dn;
> > u32 ddw_avail[3];
> > +
> > struct direct_window *window;
> > -   struct property *win64;
> > +   struct property *win64, *dfl_win;
> 
> Make it "default_win" or "def_win", "dfl" hurts to read :)

Sure, no problem :)

> 
> > struct dynamic_dma_window_prop *ddwprop;
> > struct failed_ddw_pdn *fpdn;
> >  
> > @@ -1110,8 +,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> > device_node *pdn)
> > if (ret)
> > goto out_failed;
> >  
> > -   /*
> > -* Query if there is a second window of size to map the
> > +   /*
> > +* First step of setting up DDW is removing the default DMA window,
> > +* if it's present. It will make all the resources available to the
> > +* new DDW window.
> > +* If anything fails after this, we need to restore it.
> > +*/
> > +
> > +   dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
> > +   if (dfl_win)
> > +   remove_dma_window(pdn, ddw_avail, dfl_win);
> 
> Before doing so, you want to make sure that the "reset" is actually
> supported. Thanks,

Good catch, I will improve that.

> 
> 
> > +
> > +   /*
> > +* Query if there is a window of size to map the
> >  * whole partition.  Query returns number of windows, largest
> >  * block assigned to PE (partition endpoint), and two bitmasks
> >  * of page sizes: supported and supported for migrate-dma.
> > @@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> > device_node *pdn)
> > kfree(win64);
> >  
> >  out_failed:
> > +   if (dfl_win)
> > +   reset_dma_window(dev, pdn);
> >  
> > fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
> > if (!fpdn)
> > 

Best regards,
Leonardo



Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-22 Thread Alexey Kardashevskiy



On 19/06/2020 15:06, Leonardo Bras wrote:
> On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
> default DMA window for the device, before attempting to configure a DDW,
> in order to make the maximum resources available for the next DDW to be
> created.
> 
> This is a requirement for some devices to use DDW, given they only
> allow one DMA window.
> 
> If setting up a new DDW fails anywhere after the removal of this
> default DMA window, restore it using reset_dma_window.

Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
as pHyp can only create a single window and it has to map at
0x800.... They probably do not care though.

Under KVM, this will fail as VFIO allows creating  2 windows and it
starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
the window address == 0 as a failure. And we want to keep both DMA
windows for PCI adapters with both 64bit and 32bit PCI functions (I
heard AMD GPU video + audio are like this) or someone could hotplug
32bit DMA device on a vphb with already present 64bit DMA window so we
do not remove the default window.

The last discussed thing I remember was that there was supposed to be a
new bit in "ibm,architecture-vec-5" (forgot the details), we could use
that to decide whether to keep the default window or not, like this.

> 
> Signed-off-by: Leonardo Bras 
> ---
>  arch/powerpc/platforms/pseries/iommu.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c 
> b/arch/powerpc/platforms/pseries/iommu.c
> index de633f6ae093..68d1ea957ac7 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> device_node *pdn)
>   u64 dma_addr, max_addr;
>   struct device_node *dn;
>   u32 ddw_avail[3];
> +
>   struct direct_window *window;
> - struct property *win64;
> + struct property *win64, *dfl_win;

Make it "default_win" or "def_win", "dfl" hurts to read :)

>   struct dynamic_dma_window_prop *ddwprop;
>   struct failed_ddw_pdn *fpdn;
>  
> @@ -1110,8 +,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> device_node *pdn)
>   if (ret)
>   goto out_failed;
>  
> -   /*
> -  * Query if there is a second window of size to map the
> + /*
> +  * First step of setting up DDW is removing the default DMA window,
> +  * if it's present. It will make all the resources available to the
> +  * new DDW window.
> +  * If anything fails after this, we need to restore it.
> +  */
> +
> + dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
> + if (dfl_win)
> + remove_dma_window(pdn, ddw_avail, dfl_win);

Before doing so, you want to make sure that the "reset" is actually
supported. Thanks,


> +
> + /*
> +  * Query if there is a window of size to map the
>* whole partition.  Query returns number of windows, largest
>* block assigned to PE (partition endpoint), and two bitmasks
>* of page sizes: supported and supported for migrate-dma.
> @@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
> device_node *pdn)
>   kfree(win64);
>  
>  out_failed:
> + if (dfl_win)
> + reset_dma_window(dev, pdn);
>  
>   fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
>   if (!fpdn)
> 

-- 
Alexey


Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-20 Thread kernel test robot
Hi Leonardo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.8-rc1 next-20200618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Leonardo-Bras/Remove-default-DMA-window-before-creating-DDW/20200619-131022
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r031-20200619 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 
63700971ac9cdf198faa4a3a7c226fa579e49206)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> arch/powerpc/platforms/pseries/iommu.c::6: warning: variable 'dfl_win' 
>> is used uninitialized whenever 'if' condition is true 
>> [-Wsometimes-uninitialized]
if (ret)
^~~
arch/powerpc/platforms/pseries/iommu.c:1234:6: note: uninitialized use occurs 
here
if (dfl_win)
^~~
arch/powerpc/platforms/pseries/iommu.c::2: note: remove the 'if' if its 
condition is always false
if (ret)
^~~~
arch/powerpc/platforms/pseries/iommu.c:1079:34: note: initialize the variable 
'dfl_win' to silence this warning
struct property *win64, *dfl_win;
^
= NULL
1 warning generated.

vim + arch/powerpc/platforms/pseries/iommu.c

715a454e17d328 Leonardo Bras2020-06-19  1056  
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1057  /*
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1058   * If the PE supports 
dynamic dma windows, and there is space for a table
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1059   * that can map all pages 
in a linear offset, then setup such a table,
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1060   * and record the 
dma-offset in the struct device.
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1061   *
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1062   * dev: the pci device we 
are checking
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1063   * pdn: the parent pe 
node with the ibm,dma_window property
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1064   * Future: also check if 
we can remap the base window for our base page size
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1065   *
9ae2fddeda4cbf Christoph Hellwig2019-02-13  1066   * returns the dma offset 
for use by the direct mapped DMA code.
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1067   */
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1068  static u64 
enable_ddw(struct pci_dev *dev, struct device_node *pdn)
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1069  {
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1070int len, ret;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1071struct 
ddw_query_response query;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1072struct 
ddw_create_response create;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1073int page_shift;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1074u64 dma_addr, max_addr;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1075struct device_node *dn;
9410e0185e6539 Alexey Kardashevskiy 2014-09-25  1076u32 ddw_avail[3];
3248d5f65aac44 Leonardo Bras2020-06-19  1077  
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1078struct direct_window 
*window;
3248d5f65aac44 Leonardo Bras2020-06-19  1079struct property *win64, 
*dfl_win;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1080struct 
dynamic_dma_window_prop *ddwprop;
61435690a9c781 Nishanth Aravamudan  2013-03-07  1081struct failed_ddw_pdn 
*fpdn;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1082  
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1083
mutex_lock(_window_init_mutex);
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1084  
b73a635f348610 Milton Miller2011-05-11  1085dma_addr = 
find_existing_ddw(pdn);
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1086if (dma_addr != 0)
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1087goto out_unlock;
4e8b0cf46b2570 Nishanth Aravamudan  2011-02-10  1088  
61435690a9c781 Nishanth Aravamudan  2013-03-07  1089/*
61435690a9c781 Nishanth Aravamudan  2013-03-07  1090 * If we already went 
through this for a previous function of
61435690a9c781 Nishanth Aravamudan  2013-03-07  1091 * the same device and 
failed, we don't want to muck 

[PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW

2020-06-18 Thread Leonardo Bras
On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
default DMA window for the device, before attempting to configure a DDW,
in order to make the maximum resources available for the next DDW to be
created.

This is a requirement for some devices to use DDW, given they only
allow one DMA window.

If setting up a new DDW fails anywhere after the removal of this
default DMA window, restore it using reset_dma_window.

Signed-off-by: Leonardo Bras 
---
 arch/powerpc/platforms/pseries/iommu.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c 
b/arch/powerpc/platforms/pseries/iommu.c
index de633f6ae093..68d1ea957ac7 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
device_node *pdn)
u64 dma_addr, max_addr;
struct device_node *dn;
u32 ddw_avail[3];
+
struct direct_window *window;
-   struct property *win64;
+   struct property *win64, *dfl_win;
struct dynamic_dma_window_prop *ddwprop;
struct failed_ddw_pdn *fpdn;
 
@@ -1110,8 +,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
device_node *pdn)
if (ret)
goto out_failed;
 
-   /*
-* Query if there is a second window of size to map the
+   /*
+* First step of setting up DDW is removing the default DMA window,
+* if it's present. It will make all the resources available to the
+* new DDW window.
+* If anything fails after this, we need to restore it.
+*/
+
+   dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
+   if (dfl_win)
+   remove_dma_window(pdn, ddw_avail, dfl_win);
+
+   /*
+* Query if there is a window of size to map the
 * whole partition.  Query returns number of windows, largest
 * block assigned to PE (partition endpoint), and two bitmasks
 * of page sizes: supported and supported for migrate-dma.
@@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct 
device_node *pdn)
kfree(win64);
 
 out_failed:
+   if (dfl_win)
+   reset_dma_window(dev, pdn);
 
fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
if (!fpdn)
-- 
2.25.4