diff mbox series

[v3,2/4] hw/cxl: Add utility functions decoder interleave ways and target count.

Message ID 20230911114313.6144-3-Jonathan.Cameron@huawei.com
State Superseded
Headers show
Series hw/cxl: Support emulating 4 HDM decoders throughout topology | expand

Commit Message

Jonathan Cameron Sept. 11, 2023, 11:43 a.m. UTC
As an encoded version of these key configuration parameters is available
in a register, provide functions to extract it again so as to avoid
the need for duplicating the storage.

Whilst here update the _enc() function to include additional values
as defined in the CXL 3.0 specification. Whilst they are not
currently used in the emulation, they may be in future and it is
easier to compare with the specification if all values are covered.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3: No changes, picked up tags.
v2: Thanks to Philippe Mathieu-Daudé
 - Expand both enc() and dec() functions to include full set of values
   defined in CXL r3.0
 - Pushed implementation down into the .c file.
---
 include/hw/cxl/cxl_component.h |  2 ++
 hw/cxl/cxl-component-utils.c   | 59 ++++++++++++++++++++++++++++++----
 2 files changed, 55 insertions(+), 6 deletions(-)

Comments

Fan Ni Sept. 12, 2023, 5:20 p.m. UTC | #1
On Mon, Sep 11, 2023 at 12:43:11PM +0100, Jonathan Cameron wrote:

> As an encoded version of these key configuration parameters is available
> in a register, provide functions to extract it again so as to avoid
> the need for duplicating the storage.
> 
> Whilst here update the _enc() function to include additional values
> as defined in the CXL 3.0 specification. Whilst they are not
> currently used in the emulation, they may be in future and it is
> easier to compare with the specification if all values are covered.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

LGTM. Only one minor comment inline.

Reviewed-by: Fan Ni <fan.ni@samsung.com>


> v3: No changes, picked up tags.
> v2: Thanks to Philippe Mathieu-Daudé
>  - Expand both enc() and dec() functions to include full set of values
>    defined in CXL r3.0
>  - Pushed implementation down into the .c file.
> ---
>  include/hw/cxl/cxl_component.h |  2 ++
>  hw/cxl/cxl-component-utils.c   | 59 ++++++++++++++++++++++++++++++----
>  2 files changed, 55 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> index bdb3881a6b..ef9e033919 100644
> --- a/include/hw/cxl/cxl_component.h
> +++ b/include/hw/cxl/cxl_component.h
> @@ -226,8 +226,10 @@ void cxl_component_create_dvsec(CXLComponentState *cxl_cstate,
>                                  uint16_t type, uint8_t rev, uint8_t *body);
>  
>  int cxl_decoder_count_enc(int count);
> +int cxl_decoder_count_dec(int enc_cnt);
>  
>  uint8_t cxl_interleave_ways_enc(int iw, Error **errp);
> +int cxl_interleave_ways_dec(uint8_t iw_enc, Error **errp);
>  uint8_t cxl_interleave_granularity_enc(uint64_t gran, Error **errp);
>  
>  hwaddr cxl_decode_ig(int ig);
> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> index ea2d4770ec..352d0dace2 100644
> --- a/hw/cxl/cxl-component-utils.c
> +++ b/hw/cxl/cxl-component-utils.c
> @@ -13,15 +13,45 @@
>  #include "hw/pci/pci.h"
>  #include "hw/cxl/cxl.h"
>  
> +/* CXL r3.0 Section 8.2.4.19.1 CXL HDM Decoder Capability Register */
>  int cxl_decoder_count_enc(int count)
>  {
>      switch (count) {
> -    case 1: return 0;
> -    case 2: return 1;
> -    case 4: return 2;
> -    case 6: return 3;
> -    case 8: return 4;
> -    case 10: return 5;
> +    case 1: return 0x0;
> +    case 2: return 0x1;
> +    case 4: return 0x2;
> +    case 6: return 0x3;
> +    case 8: return 0x4;
> +    case 10: return 0x5;
> +    /* Switches and Host Bridges may have more than 10 decoders */
> +    case 12: return 0x6;
> +    case 14: return 0x7;
> +    case 16: return 0x8;
> +    case 20: return 0x9;
> +    case 24: return 0xa;
> +    case 28: return 0xb;
> +    case 32: return 0xc;
> +    }
> +    return 0;
> +}
> +
> +int cxl_decoder_count_dec(int enc_cnt)
> +{
> +    switch (enc_cnt) {
> +    case 0x0: return 1;
> +    case 0x1: return 2;
> +    case 0x2: return 4;
> +    case 0x3: return 6;
> +    case 0x4: return 8;
> +    case 0x5: return 10;
> +    /* Switches and Host Bridges may have more than 10 decoders */
> +    case 0x6: return 12;
> +    case 0x7: return 14;
> +    case 0x8: return 16;
> +    case 0x9: return 20;
> +    case 0xa: return 24;
> +    case 0xb: return 28;
> +    case 0xc: return 32;
>      }
>      return 0;
>  }
> @@ -410,6 +440,23 @@ uint8_t cxl_interleave_ways_enc(int iw, Error **errp)
>      }
>  }
>  

Similar as decoder count dec/enc, maybe we want to add a line of comment below.
/* CXL r3.0 Section 8.2.4.19.7 CXL HDM Decoder n Control Register */

Fan
> +int cxl_interleave_ways_dec(uint8_t iw_enc, Error **errp)
> +{
> +    switch (iw_enc) {
> +    case 0x0: return 1;
> +    case 0x1: return 2;
> +    case 0x2: return 4;
> +    case 0x3: return 8;
> +    case 0x4: return 16;
> +    case 0x8: return 3;
> +    case 0x9: return 6;
> +    case 0xa: return 12;
> +    default:
> +        error_setg(errp, "Encoded interleave ways: %d not supported", iw_enc);
> +        return 0;
> +    }
> +}
> +
>  uint8_t cxl_interleave_granularity_enc(uint64_t gran, Error **errp)
>  {
>      switch (gran) {
> -- 
> 2.39.2
> 
>
Jonathan Cameron Sept. 13, 2023, 8:58 a.m. UTC | #2
On Tue, 12 Sep 2023 17:20:05 +0000
Fan Ni <fan.ni@samsung.com> wrote:

> On Mon, Sep 11, 2023 at 12:43:11PM +0100, Jonathan Cameron wrote:
> 
> > As an encoded version of these key configuration parameters is available
> > in a register, provide functions to extract it again so as to avoid
> > the need for duplicating the storage.
> > 
> > Whilst here update the _enc() function to include additional values
> > as defined in the CXL 3.0 specification. Whilst they are not
> > currently used in the emulation, they may be in future and it is
> > easier to compare with the specification if all values are covered.
> > 
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---  
> 
> LGTM. Only one minor comment inline.
> 
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
Thanks!

> > +
> > +int cxl_decoder_count_dec(int enc_cnt)
> > +{
> > +    switch (enc_cnt) {
> > +    case 0x0: return 1;
> > +    case 0x1: return 2;
> > +    case 0x2: return 4;
> > +    case 0x3: return 6;
> > +    case 0x4: return 8;
> > +    case 0x5: return 10;
> > +    /* Switches and Host Bridges may have more than 10 decoders */
> > +    case 0x6: return 12;
> > +    case 0x7: return 14;
> > +    case 0x8: return 16;
> > +    case 0x9: return 20;
> > +    case 0xa: return 24;
> > +    case 0xb: return 28;
> > +    case 0xc: return 32;
> >      }
> >      return 0;
> >  }
> > @@ -410,6 +440,23 @@ uint8_t cxl_interleave_ways_enc(int iw, Error **errp)
> >      }
> >  }
> >    
> 
> Similar as decoder count dec/enc, maybe we want to add a line of comment below.
> /* CXL r3.0 Section 8.2.4.19.7 CXL HDM Decoder n Control Register */

I'll do it before cxl_interleave_ways_enc() - one function up in the file
as applies equally well there.
> 
> Fan
> > +int cxl_interleave_ways_dec(uint8_t iw_enc, Error **errp)
> > +{
> > +    switch (iw_enc) {
> > +    case 0x0: return 1;
> > +    case 0x1: return 2;
> > +    case 0x2: return 4;
> > +    case 0x3: return 8;
> > +    case 0x4: return 16;
> > +    case 0x8: return 3;
> > +    case 0x9: return 6;
> > +    case 0xa: return 12;
> > +    default:
> > +        error_setg(errp, "Encoded interleave ways: %d not supported", iw_enc);
> > +        return 0;
> > +    }
> > +}
> > +
> >  uint8_t cxl_interleave_granularity_enc(uint64_t gran, Error **errp)
> >  {
> >      switch (gran) {
> > -- 
> > 2.39.2
> > 
> >
diff mbox series

Patch

diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index bdb3881a6b..ef9e033919 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -226,8 +226,10 @@  void cxl_component_create_dvsec(CXLComponentState *cxl_cstate,
                                 uint16_t type, uint8_t rev, uint8_t *body);
 
 int cxl_decoder_count_enc(int count);
+int cxl_decoder_count_dec(int enc_cnt);
 
 uint8_t cxl_interleave_ways_enc(int iw, Error **errp);
+int cxl_interleave_ways_dec(uint8_t iw_enc, Error **errp);
 uint8_t cxl_interleave_granularity_enc(uint64_t gran, Error **errp);
 
 hwaddr cxl_decode_ig(int ig);
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index ea2d4770ec..352d0dace2 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -13,15 +13,45 @@ 
 #include "hw/pci/pci.h"
 #include "hw/cxl/cxl.h"
 
+/* CXL r3.0 Section 8.2.4.19.1 CXL HDM Decoder Capability Register */
 int cxl_decoder_count_enc(int count)
 {
     switch (count) {
-    case 1: return 0;
-    case 2: return 1;
-    case 4: return 2;
-    case 6: return 3;
-    case 8: return 4;
-    case 10: return 5;
+    case 1: return 0x0;
+    case 2: return 0x1;
+    case 4: return 0x2;
+    case 6: return 0x3;
+    case 8: return 0x4;
+    case 10: return 0x5;
+    /* Switches and Host Bridges may have more than 10 decoders */
+    case 12: return 0x6;
+    case 14: return 0x7;
+    case 16: return 0x8;
+    case 20: return 0x9;
+    case 24: return 0xa;
+    case 28: return 0xb;
+    case 32: return 0xc;
+    }
+    return 0;
+}
+
+int cxl_decoder_count_dec(int enc_cnt)
+{
+    switch (enc_cnt) {
+    case 0x0: return 1;
+    case 0x1: return 2;
+    case 0x2: return 4;
+    case 0x3: return 6;
+    case 0x4: return 8;
+    case 0x5: return 10;
+    /* Switches and Host Bridges may have more than 10 decoders */
+    case 0x6: return 12;
+    case 0x7: return 14;
+    case 0x8: return 16;
+    case 0x9: return 20;
+    case 0xa: return 24;
+    case 0xb: return 28;
+    case 0xc: return 32;
     }
     return 0;
 }
@@ -410,6 +440,23 @@  uint8_t cxl_interleave_ways_enc(int iw, Error **errp)
     }
 }
 
+int cxl_interleave_ways_dec(uint8_t iw_enc, Error **errp)
+{
+    switch (iw_enc) {
+    case 0x0: return 1;
+    case 0x1: return 2;
+    case 0x2: return 4;
+    case 0x3: return 8;
+    case 0x4: return 16;
+    case 0x8: return 3;
+    case 0x9: return 6;
+    case 0xa: return 12;
+    default:
+        error_setg(errp, "Encoded interleave ways: %d not supported", iw_enc);
+        return 0;
+    }
+}
+
 uint8_t cxl_interleave_granularity_enc(uint64_t gran, Error **errp)
 {
     switch (gran) {