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.0000.0000.0000. 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 <leobra...@gmail.com>
> > ---
> > 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 +1111,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
