[2/2] phb3/phb4/p7ioc: Document supported TCE sizes in DT

Message ID 20180205040008.28362-2-ruscur@russell.cc
State Accepted
Headers show
Series
  • [1/2] phb4: Fix TCE page size
Related show

Commit Message

Russell Currey Feb. 5, 2018, 4 a.m.
Add a new property, "ibm,supported-tce-sizes", to advertise to Linux how
big the available TCE sizes are.  Each value is a bit shift, from
smallest to largest.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 hw/p7ioc-phb.c | 5 +++++
 hw/phb3.c      | 5 +++++
 hw/phb4.c      | 5 +++++
 3 files changed, 15 insertions(+)

Comments

Alexey Kardashevskiy Feb. 5, 2018, 5:02 a.m. | #1
On 05/02/18 15:00, Russell Currey wrote:
> Add a new property, "ibm,supported-tce-sizes", to advertise to Linux how
> big the available TCE sizes are.  Each value is a bit shift, from
> smallest to largest.

Why would we possibly need such a limit? We already have an example of DDW
in sPAPR where possible IOMMU page sizes are fixed and there is no way to
have 2M IOMMU pages in the guest, now the same sort of limit is being added
to skiboot...


> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
>  hw/p7ioc-phb.c | 5 +++++
>  hw/phb3.c      | 5 +++++
>  hw/phb4.c      | 5 +++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/hw/p7ioc-phb.c b/hw/p7ioc-phb.c
> index 6c074518..b0a26e84 100644
> --- a/hw/p7ioc-phb.c
> +++ b/hw/p7ioc-phb.c
> @@ -2596,6 +2596,11 @@ static void p7ioc_pcie_add_node(struct p7ioc_phb *p)
>  	tkill = reg[0] + PHB_TCE_KILL;
>  	dt_add_property_cells(np, "ibm,opal-tce-kill",
>  			      hi32(tkill), lo32(tkill));
> +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> +			      12, // 4K
> +			      16, // 64K
> +			      24, // 16M
> +			      34); // 16G
>  
>  	/*
>  	 * Linux may use this property to allocate the diag data buffer, which
> diff --git a/hw/phb3.c b/hw/phb3.c
> index 0b4c3c32..0c4e1eb3 100644
> --- a/hw/phb3.c
> +++ b/hw/phb3.c
> @@ -4469,6 +4469,11 @@ static void phb3_add_properties(struct phb3 *p)
>  	tkill = reg + PHB_TCE_KILL;
>  	dt_add_property_cells(np, "ibm,opal-tce-kill",
>  			      hi32(tkill), lo32(tkill));
> +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> +			      12, // 4K
> +			      16, // 64K
> +			      24, // 16M
> +			      28); // 256M
>  
>  	/*
>  	 * Indicate to Linux that the architected IODA2 MSI EOI method
> diff --git a/hw/phb4.c b/hw/phb4.c
> index 0a7e3065..946f9ab6 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -4688,6 +4688,11 @@ static void phb4_add_properties(struct phb4 *p)
>  	/* M64 ranges start at 1 as MBT0 is used for M32 */
>  	dt_add_property_cells(np, "ibm,opal-available-m64-ranges",
>  			      1, p->mbt_size - 1);
> +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> +			      12, // 4K
> +			      16, // 64K
> +			      21, // 2M
> +			      30); // 1G
>  
>  	/* Tell Linux about alignment limits for segment splits.
>  	 *
>
Russell Currey Feb. 5, 2018, 5:07 a.m. | #2
On Mon, 2018-02-05 at 16:02 +1100, Alexey Kardashevskiy wrote:
> On 05/02/18 15:00, Russell Currey wrote:
> > Add a new property, "ibm,supported-tce-sizes", to advertise to
> > Linux how
> > big the available TCE sizes are.  Each value is a bit shift, from
> > smallest to largest.
> 
> Why would we possibly need such a limit? We already have an example
> of DDW
> in sPAPR where possible IOMMU page sizes are fixed and there is no
> way to
> have 2M IOMMU pages in the guest, now the same sort of limit is being
> added
> to skiboot...

There are no limits being imposed, this is purely documenting what the
hardware supports.
> 
> 
> > 
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> >  hw/p7ioc-phb.c | 5 +++++
> >  hw/phb3.c      | 5 +++++
> >  hw/phb4.c      | 5 +++++
> >  3 files changed, 15 insertions(+)
> > 
> > diff --git a/hw/p7ioc-phb.c b/hw/p7ioc-phb.c
> > index 6c074518..b0a26e84 100644
> > --- a/hw/p7ioc-phb.c
> > +++ b/hw/p7ioc-phb.c
> > @@ -2596,6 +2596,11 @@ static void p7ioc_pcie_add_node(struct
> > p7ioc_phb *p)
> >  	tkill = reg[0] + PHB_TCE_KILL;
> >  	dt_add_property_cells(np, "ibm,opal-tce-kill",
> >  			      hi32(tkill), lo32(tkill));
> > +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> > +			      12, // 4K
> > +			      16, // 64K
> > +			      24, // 16M
> > +			      34); // 16G
> >  
> >  	/*
> >  	 * Linux may use this property to allocate the diag data
> > buffer, which
> > diff --git a/hw/phb3.c b/hw/phb3.c
> > index 0b4c3c32..0c4e1eb3 100644
> > --- a/hw/phb3.c
> > +++ b/hw/phb3.c
> > @@ -4469,6 +4469,11 @@ static void phb3_add_properties(struct phb3
> > *p)
> >  	tkill = reg + PHB_TCE_KILL;
> >  	dt_add_property_cells(np, "ibm,opal-tce-kill",
> >  			      hi32(tkill), lo32(tkill));
> > +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> > +			      12, // 4K
> > +			      16, // 64K
> > +			      24, // 16M
> > +			      28); // 256M
> >  
> >  	/*
> >  	 * Indicate to Linux that the architected IODA2 MSI EOI
> > method
> > diff --git a/hw/phb4.c b/hw/phb4.c
> > index 0a7e3065..946f9ab6 100644
> > --- a/hw/phb4.c
> > +++ b/hw/phb4.c
> > @@ -4688,6 +4688,11 @@ static void phb4_add_properties(struct phb4
> > *p)
> >  	/* M64 ranges start at 1 as MBT0 is used for M32 */
> >  	dt_add_property_cells(np, "ibm,opal-available-m64-ranges",
> >  			      1, p->mbt_size - 1);
> > +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> > +			      12, // 4K
> > +			      16, // 64K
> > +			      21, // 2M
> > +			      30); // 1G
> >  
> >  	/* Tell Linux about alignment limits for segment splits.
> >  	 *
> > 
> 
>
Alexey Kardashevskiy Feb. 5, 2018, 5:52 a.m. | #3
On 05/02/18 16:07, Russell Currey wrote:
> On Mon, 2018-02-05 at 16:02 +1100, Alexey Kardashevskiy wrote:
>> On 05/02/18 15:00, Russell Currey wrote:
>>> Add a new property, "ibm,supported-tce-sizes", to advertise to
>>> Linux how
>>> big the available TCE sizes are.  Each value is a bit shift, from
>>> smallest to largest.
>>
>> Why would we possibly need such a limit? We already have an example
>> of DDW
>> in sPAPR where possible IOMMU page sizes are fixed and there is no
>> way to
>> have 2M IOMMU pages in the guest, now the same sort of limit is being
>> added
>> to skiboot...
> 
> There are no limits being imposed, this is purely documenting what the
> hardware supports.


Wow. Just found it in the spec. Pls put this kind of information to the
commit log as it is a surprise that now I cannot get 16M IOMMU pages on
P9/hash.




>>
>>
>>>
>>> Signed-off-by: Russell Currey <ruscur@russell.cc>
>>> ---
>>>  hw/p7ioc-phb.c | 5 +++++
>>>  hw/phb3.c      | 5 +++++
>>>  hw/phb4.c      | 5 +++++
>>>  3 files changed, 15 insertions(+)
>>>
>>> diff --git a/hw/p7ioc-phb.c b/hw/p7ioc-phb.c
>>> index 6c074518..b0a26e84 100644
>>> --- a/hw/p7ioc-phb.c
>>> +++ b/hw/p7ioc-phb.c
>>> @@ -2596,6 +2596,11 @@ static void p7ioc_pcie_add_node(struct
>>> p7ioc_phb *p)
>>>  	tkill = reg[0] + PHB_TCE_KILL;
>>>  	dt_add_property_cells(np, "ibm,opal-tce-kill",
>>>  			      hi32(tkill), lo32(tkill));
>>> +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
>>> +			      12, // 4K
>>> +			      16, // 64K
>>> +			      24, // 16M
>>> +			      34); // 16G
>>>  
>>>  	/*
>>>  	 * Linux may use this property to allocate the diag data
>>> buffer, which
>>> diff --git a/hw/phb3.c b/hw/phb3.c
>>> index 0b4c3c32..0c4e1eb3 100644
>>> --- a/hw/phb3.c
>>> +++ b/hw/phb3.c
>>> @@ -4469,6 +4469,11 @@ static void phb3_add_properties(struct phb3
>>> *p)
>>>  	tkill = reg + PHB_TCE_KILL;
>>>  	dt_add_property_cells(np, "ibm,opal-tce-kill",
>>>  			      hi32(tkill), lo32(tkill));
>>> +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
>>> +			      12, // 4K
>>> +			      16, // 64K
>>> +			      24, // 16M
>>> +			      28); // 256M
>>>  
>>>  	/*
>>>  	 * Indicate to Linux that the architected IODA2 MSI EOI
>>> method
>>> diff --git a/hw/phb4.c b/hw/phb4.c
>>> index 0a7e3065..946f9ab6 100644
>>> --- a/hw/phb4.c
>>> +++ b/hw/phb4.c
>>> @@ -4688,6 +4688,11 @@ static void phb4_add_properties(struct phb4
>>> *p)
>>>  	/* M64 ranges start at 1 as MBT0 is used for M32 */
>>>  	dt_add_property_cells(np, "ibm,opal-available-m64-ranges",
>>>  			      1, p->mbt_size - 1);
>>> +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
>>> +			      12, // 4K
>>> +			      16, // 64K
>>> +			      21, // 2M
>>> +			      30); // 1G
>>>  
>>>  	/* Tell Linux about alignment limits for segment splits.
>>>  	 *
>>>
>>
>>
Benjamin Herrenschmidt Feb. 5, 2018, 10:32 a.m. | #4
On Mon, 2018-02-05 at 16:52 +1100, Alexey Kardashevskiy wrote:
> On 05/02/18 16:07, Russell Currey wrote:
> > On Mon, 2018-02-05 at 16:02 +1100, Alexey Kardashevskiy wrote:
> > > On 05/02/18 15:00, Russell Currey wrote:
> > > > Add a new property, "ibm,supported-tce-sizes", to advertise to
> > > > Linux how
> > > > big the available TCE sizes are.  Each value is a bit shift, from
> > > > smallest to largest.
> > > 
> > > Why would we possibly need such a limit? We already have an example
> > > of DDW
> > > in sPAPR where possible IOMMU page sizes are fixed and there is no
> > > way to
> > > have 2M IOMMU pages in the guest, now the same sort of limit is being
> > > added
> > > to skiboot...
> > 
> > There are no limits being imposed, this is purely documenting what the
> > hardware supports.
> 
> 
> Wow. Just found it in the spec. Pls put this kind of information to the
> commit log as it is a surprise that now I cannot get 16M IOMMU pages on
> P9/hash.

According to the spec at least, you'd have to use multiples 2M pages.
I've asked the HW guys to confirm.

Cheers,
Ben.
> 
> 
> 
> > > 
> > > 
> > > > 
> > > > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > > > ---
> > > >  hw/p7ioc-phb.c | 5 +++++
> > > >  hw/phb3.c      | 5 +++++
> > > >  hw/phb4.c      | 5 +++++
> > > >  3 files changed, 15 insertions(+)
> > > > 
> > > > diff --git a/hw/p7ioc-phb.c b/hw/p7ioc-phb.c
> > > > index 6c074518..b0a26e84 100644
> > > > --- a/hw/p7ioc-phb.c
> > > > +++ b/hw/p7ioc-phb.c
> > > > @@ -2596,6 +2596,11 @@ static void p7ioc_pcie_add_node(struct
> > > > p7ioc_phb *p)
> > > >  	tkill = reg[0] + PHB_TCE_KILL;
> > > >  	dt_add_property_cells(np, "ibm,opal-tce-kill",
> > > >  			      hi32(tkill), lo32(tkill));
> > > > +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> > > > +			      12, // 4K
> > > > +			      16, // 64K
> > > > +			      24, // 16M
> > > > +			      34); // 16G
> > > >  
> > > >  	/*
> > > >  	 * Linux may use this property to allocate the diag data
> > > > buffer, which
> > > > diff --git a/hw/phb3.c b/hw/phb3.c
> > > > index 0b4c3c32..0c4e1eb3 100644
> > > > --- a/hw/phb3.c
> > > > +++ b/hw/phb3.c
> > > > @@ -4469,6 +4469,11 @@ static void phb3_add_properties(struct phb3
> > > > *p)
> > > >  	tkill = reg + PHB_TCE_KILL;
> > > >  	dt_add_property_cells(np, "ibm,opal-tce-kill",
> > > >  			      hi32(tkill), lo32(tkill));
> > > > +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> > > > +			      12, // 4K
> > > > +			      16, // 64K
> > > > +			      24, // 16M
> > > > +			      28); // 256M
> > > >  
> > > >  	/*
> > > >  	 * Indicate to Linux that the architected IODA2 MSI EOI
> > > > method
> > > > diff --git a/hw/phb4.c b/hw/phb4.c
> > > > index 0a7e3065..946f9ab6 100644
> > > > --- a/hw/phb4.c
> > > > +++ b/hw/phb4.c
> > > > @@ -4688,6 +4688,11 @@ static void phb4_add_properties(struct phb4
> > > > *p)
> > > >  	/* M64 ranges start at 1 as MBT0 is used for M32 */
> > > >  	dt_add_property_cells(np, "ibm,opal-available-m64-ranges",
> > > >  			      1, p->mbt_size - 1);
> > > > +	dt_add_property_cells(np, "ibm,supported-tce-sizes",
> > > > +			      12, // 4K
> > > > +			      16, // 64K
> > > > +			      21, // 2M
> > > > +			      30); // 1G
> > > >  
> > > >  	/* Tell Linux about alignment limits for segment splits.
> > > >  	 *
> > > > 
> > > 
> > > 
> 
>

Patch

diff --git a/hw/p7ioc-phb.c b/hw/p7ioc-phb.c
index 6c074518..b0a26e84 100644
--- a/hw/p7ioc-phb.c
+++ b/hw/p7ioc-phb.c
@@ -2596,6 +2596,11 @@  static void p7ioc_pcie_add_node(struct p7ioc_phb *p)
 	tkill = reg[0] + PHB_TCE_KILL;
 	dt_add_property_cells(np, "ibm,opal-tce-kill",
 			      hi32(tkill), lo32(tkill));
+	dt_add_property_cells(np, "ibm,supported-tce-sizes",
+			      12, // 4K
+			      16, // 64K
+			      24, // 16M
+			      34); // 16G
 
 	/*
 	 * Linux may use this property to allocate the diag data buffer, which
diff --git a/hw/phb3.c b/hw/phb3.c
index 0b4c3c32..0c4e1eb3 100644
--- a/hw/phb3.c
+++ b/hw/phb3.c
@@ -4469,6 +4469,11 @@  static void phb3_add_properties(struct phb3 *p)
 	tkill = reg + PHB_TCE_KILL;
 	dt_add_property_cells(np, "ibm,opal-tce-kill",
 			      hi32(tkill), lo32(tkill));
+	dt_add_property_cells(np, "ibm,supported-tce-sizes",
+			      12, // 4K
+			      16, // 64K
+			      24, // 16M
+			      28); // 256M
 
 	/*
 	 * Indicate to Linux that the architected IODA2 MSI EOI method
diff --git a/hw/phb4.c b/hw/phb4.c
index 0a7e3065..946f9ab6 100644
--- a/hw/phb4.c
+++ b/hw/phb4.c
@@ -4688,6 +4688,11 @@  static void phb4_add_properties(struct phb4 *p)
 	/* M64 ranges start at 1 as MBT0 is used for M32 */
 	dt_add_property_cells(np, "ibm,opal-available-m64-ranges",
 			      1, p->mbt_size - 1);
+	dt_add_property_cells(np, "ibm,supported-tce-sizes",
+			      12, // 4K
+			      16, // 64K
+			      21, // 2M
+			      30); // 1G
 
 	/* Tell Linux about alignment limits for segment splits.
 	 *