All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
@ 2022-06-07 16:07 ` Jonathan Cameron via
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-06-07 16:07 UTC (permalink / raw)
  To: qemu-devel, Michael S . Tsirkin, Ben Widawsky
  Cc: Paolo Bonzini, linux-cxl, linuxarm, alex.bennee,
	Marcel Apfelbaum, Igor Mammedov, Markus Armbruster,
	Mark Cave-Ayland, Adam Manzanares, Tong Zhang,
	Shameerali Kolothum Thodi

Without being able to write these registers, no interleaving is possible.
More refined checks of HDM register state on commit to follow.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3: Actually pass the parameter to the call...
v2: (Ben Widawsky)
- Correctly set a tighter write mask for the endpoint devices where this
  register has a different use.
  
 hw/cxl/cxl-component-utils.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 7985c9bfca..2208284ee6 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
     reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
 }
 
-static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
+static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
+                            enum reg_type type)
 {
     int decoder_count = 1;
     int i;
@@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
         write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf0000000;
         write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0xffffffff;
         write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
+        if (type == CXL2_DEVICE) {
+            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xf0000000;
+        } else {
+            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xffffffff;
+        }
+        write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0xffffffff;
     }
 }
 
@@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t *reg_state, uint32_t *write_msk
     }
 
     init_cap_reg(HDM, 5, 1);
-    hdm_init_common(reg_state, write_msk);
+    hdm_init_common(reg_state, write_msk, type);
 
     if (caps < 5) {
         return;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
@ 2022-06-07 16:07 ` Jonathan Cameron via
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron via @ 2022-06-07 16:07 UTC (permalink / raw)
  To: qemu-devel, Michael S . Tsirkin, Ben Widawsky
  Cc: Paolo Bonzini, linux-cxl, linuxarm, alex.bennee,
	Marcel Apfelbaum, Igor Mammedov, Markus Armbruster,
	Mark Cave-Ayland, Adam Manzanares, Tong Zhang,
	Shameerali Kolothum Thodi

Without being able to write these registers, no interleaving is possible.
More refined checks of HDM register state on commit to follow.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v3: Actually pass the parameter to the call...
v2: (Ben Widawsky)
- Correctly set a tighter write mask for the endpoint devices where this
  register has a different use.
  
 hw/cxl/cxl-component-utils.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 7985c9bfca..2208284ee6 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
     reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
 }
 
-static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
+static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
+                            enum reg_type type)
 {
     int decoder_count = 1;
     int i;
@@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
         write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf0000000;
         write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0xffffffff;
         write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
+        if (type == CXL2_DEVICE) {
+            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xf0000000;
+        } else {
+            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xffffffff;
+        }
+        write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0xffffffff;
     }
 }
 
@@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t *reg_state, uint32_t *write_msk
     }
 
     init_cap_reg(HDM, 5, 1);
-    hdm_init_common(reg_state, write_msk);
+    hdm_init_common(reg_state, write_msk, type);
 
     if (caps < 5) {
         return;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
  2022-06-07 16:07 ` Jonathan Cameron via
  (?)
@ 2022-06-07 16:19 ` Ben Widawsky
  2022-06-07 16:37     ` Jonathan Cameron via
  -1 siblings, 1 reply; 6+ messages in thread
From: Ben Widawsky @ 2022-06-07 16:19 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: qemu-devel, Michael S . Tsirkin, Paolo Bonzini, linux-cxl,
	linuxarm, alex.bennee, Marcel Apfelbaum, Igor Mammedov,
	Markus Armbruster, Mark Cave-Ayland, Adam Manzanares, Tong Zhang,
	Shameerali Kolothum Thodi

On 22-06-07 17:07:47, Jonathan Cameron wrote:
> Without being able to write these registers, no interleaving is possible.
> More refined checks of HDM register state on commit to follow.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v3: Actually pass the parameter to the call...
> v2: (Ben Widawsky)
> - Correctly set a tighter write mask for the endpoint devices where this
>   register has a different use.
>   
>  hw/cxl/cxl-component-utils.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> index 7985c9bfca..2208284ee6 100644
> --- a/hw/cxl/cxl-component-utils.c
> +++ b/hw/cxl/cxl-component-utils.c
> @@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
>      reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
>  }
>  
> -static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> +static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> +                            enum reg_type type)
>  {
>      int decoder_count = 1;
>      int i;
> @@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
>          write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf0000000;
>          write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0xffffffff;
>          write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
> +        if (type == CXL2_DEVICE) {
> +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xf0000000;
> +        } else {
> +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xffffffff;
> +        }
> +        write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0xffffffff;

Should it be (type == CXL2_DEVICE || type == CXL2_TYPE3_DEVICE) ?

Otherwise,
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

>      }
>  }
>  
> @@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t *reg_state, uint32_t *write_msk
>      }
>  
>      init_cap_reg(HDM, 5, 1);
> -    hdm_init_common(reg_state, write_msk);
> +    hdm_init_common(reg_state, write_msk, type);
>  
>      if (caps < 5) {
>          return;
> -- 
> 2.32.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
  2022-06-07 16:19 ` Ben Widawsky
@ 2022-06-07 16:37     ` Jonathan Cameron via
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-06-07 16:37 UTC (permalink / raw)
  To: Ben Widawsky
  Cc: qemu-devel, Michael S . Tsirkin, Paolo Bonzini, linux-cxl,
	linuxarm, alex.bennee, Marcel Apfelbaum, Igor Mammedov,
	Markus Armbruster, Mark Cave-Ayland, Adam Manzanares, Tong Zhang,
	Shameerali Kolothum Thodi

On Tue, 7 Jun 2022 09:19:28 -0700
Ben Widawsky <bwidawsk@kernel.org> wrote:

> On 22-06-07 17:07:47, Jonathan Cameron wrote:
> > Without being able to write these registers, no interleaving is possible.
> > More refined checks of HDM register state on commit to follow.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v3: Actually pass the parameter to the call...
> > v2: (Ben Widawsky)
> > - Correctly set a tighter write mask for the endpoint devices where this
> >   register has a different use.
> >   
> >  hw/cxl/cxl-component-utils.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > index 7985c9bfca..2208284ee6 100644
> > --- a/hw/cxl/cxl-component-utils.c
> > +++ b/hw/cxl/cxl-component-utils.c
> > @@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
> >      reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
> >  }
> >  
> > -static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> > +static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> > +                            enum reg_type type)
> >  {
> >      int decoder_count = 1;
> >      int i;
> > @@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> >          write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf0000000;
> >          write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0xffffffff;
> >          write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
> > +        if (type == CXL2_DEVICE) {
> > +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xf0000000;
> > +        } else {
> > +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xffffffff;
> > +        }
> > +        write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0xffffffff;  
> 
> Should it be (type == CXL2_DEVICE || type == CXL2_TYPE3_DEVICE) ?

Good point, but also for consistency I think we need 
type == CXL2_LOGICAL_DEVICE as well.

We will only exercise the match to CXL2_TYPE3_DEVICE currently
as we don't have any emulation for MLDs (and hence LD) or type 1/2 devices
(CXL2_DEVICE).

I'll send a v4 out tomorrow.

> 
> Otherwise,
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> 
> >      }
> >  }
> >  
> > @@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t *reg_state, uint32_t *write_msk
> >      }
> >  
> >      init_cap_reg(HDM, 5, 1);
> > -    hdm_init_common(reg_state, write_msk);
> > +    hdm_init_common(reg_state, write_msk, type);
> >  
> >      if (caps < 5) {
> >          return;
> > -- 
> > 2.32.0
> >   


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
@ 2022-06-07 16:37     ` Jonathan Cameron via
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron via @ 2022-06-07 16:37 UTC (permalink / raw)
  To: Ben Widawsky
  Cc: qemu-devel, Michael S . Tsirkin, Paolo Bonzini, linux-cxl,
	linuxarm, alex.bennee, Marcel Apfelbaum, Igor Mammedov,
	Markus Armbruster, Mark Cave-Ayland, Adam Manzanares, Tong Zhang,
	Shameerali Kolothum Thodi

On Tue, 7 Jun 2022 09:19:28 -0700
Ben Widawsky <bwidawsk@kernel.org> wrote:

> On 22-06-07 17:07:47, Jonathan Cameron wrote:
> > Without being able to write these registers, no interleaving is possible.
> > More refined checks of HDM register state on commit to follow.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v3: Actually pass the parameter to the call...
> > v2: (Ben Widawsky)
> > - Correctly set a tighter write mask for the endpoint devices where this
> >   register has a different use.
> >   
> >  hw/cxl/cxl-component-utils.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > index 7985c9bfca..2208284ee6 100644
> > --- a/hw/cxl/cxl-component-utils.c
> > +++ b/hw/cxl/cxl-component-utils.c
> > @@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
> >      reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
> >  }
> >  
> > -static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> > +static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> > +                            enum reg_type type)
> >  {
> >      int decoder_count = 1;
> >      int i;
> > @@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> >          write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf0000000;
> >          write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0xffffffff;
> >          write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
> > +        if (type == CXL2_DEVICE) {
> > +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xf0000000;
> > +        } else {
> > +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xffffffff;
> > +        }
> > +        write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0xffffffff;  
> 
> Should it be (type == CXL2_DEVICE || type == CXL2_TYPE3_DEVICE) ?

Good point, but also for consistency I think we need 
type == CXL2_LOGICAL_DEVICE as well.

We will only exercise the match to CXL2_TYPE3_DEVICE currently
as we don't have any emulation for MLDs (and hence LD) or type 1/2 devices
(CXL2_DEVICE).

I'll send a v4 out tomorrow.

> 
> Otherwise,
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> 
> >      }
> >  }
> >  
> > @@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t *reg_state, uint32_t *write_msk
> >      }
> >  
> >      init_cap_reg(HDM, 5, 1);
> > -    hdm_init_common(reg_state, write_msk);
> > +    hdm_init_common(reg_state, write_msk, type);
> >  
> >      if (caps < 5) {
> >          return;
> > -- 
> > 2.32.0
> >   



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
  2022-06-07 16:37     ` Jonathan Cameron via
  (?)
@ 2022-06-07 16:41     ` Ben Widawsky
  -1 siblings, 0 replies; 6+ messages in thread
From: Ben Widawsky @ 2022-06-07 16:41 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: qemu-devel, Michael S . Tsirkin, Paolo Bonzini, linux-cxl,
	linuxarm, alex.bennee, Marcel Apfelbaum, Igor Mammedov,
	Markus Armbruster, Mark Cave-Ayland, Adam Manzanares, Tong Zhang,
	Shameerali Kolothum Thodi

On 22-06-07 17:37:02, Jonathan Cameron wrote:
> On Tue, 7 Jun 2022 09:19:28 -0700
> Ben Widawsky <bwidawsk@kernel.org> wrote:
> 
> > On 22-06-07 17:07:47, Jonathan Cameron wrote:
> > > Without being able to write these registers, no interleaving is possible.
> > > More refined checks of HDM register state on commit to follow.
> > > 
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > ---
> > > v3: Actually pass the parameter to the call...
> > > v2: (Ben Widawsky)
> > > - Correctly set a tighter write mask for the endpoint devices where this
> > >   register has a different use.
> > >   
> > >  hw/cxl/cxl-component-utils.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > > index 7985c9bfca..2208284ee6 100644
> > > --- a/hw/cxl/cxl-component-utils.c
> > > +++ b/hw/cxl/cxl-component-utils.c
> > > @@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
> > >      reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
> > >  }
> > >  
> > > -static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> > > +static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> > > +                            enum reg_type type)
> > >  {
> > >      int decoder_count = 1;
> > >      int i;
> > > @@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> > >          write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf0000000;
> > >          write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0xffffffff;
> > >          write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
> > > +        if (type == CXL2_DEVICE) {
> > > +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xf0000000;
> > > +        } else {
> > > +            write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0xffffffff;
> > > +        }
> > > +        write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0xffffffff;  
> > 
> > Should it be (type == CXL2_DEVICE || type == CXL2_TYPE3_DEVICE) ?
> 
> Good point, but also for consistency I think we need 
> type == CXL2_LOGICAL_DEVICE as well.

I was looking at this and I am not sure, but I defer to you.

> 
> We will only exercise the match to CXL2_TYPE3_DEVICE currently
> as we don't have any emulation for MLDs (and hence LD) or type 1/2 devices
> (CXL2_DEVICE).
> 
> I'll send a v4 out tomorrow.
> 

Sounds good, feel free to keep the r-b tag.

> > 
> > Otherwise,
> > Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > >      }
> > >  }
> > >  
> > > @@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t *reg_state, uint32_t *write_msk
> > >      }
> > >  
> > >      init_cap_reg(HDM, 5, 1);
> > > -    hdm_init_common(reg_state, write_msk);
> > > +    hdm_init_common(reg_state, write_msk, type);
> > >  
> > >      if (caps < 5) {
> > >          return;
> > > -- 
> > > 2.32.0
> > >   
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-07 16:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 16:07 [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers Jonathan Cameron
2022-06-07 16:07 ` Jonathan Cameron via
2022-06-07 16:19 ` Ben Widawsky
2022-06-07 16:37   ` Jonathan Cameron
2022-06-07 16:37     ` Jonathan Cameron via
2022-06-07 16:41     ` Ben Widawsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.