[PATCH 1/3] hw/cxl: Get rid of unused cfmw_list

Jonathan Cameron via posted 3 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH 1/3] hw/cxl: Get rid of unused cfmw_list
Posted by Jonathan Cameron via 2 months, 2 weeks ago
From: Li Zhijian <lizhijian@fujitsu.com>

There is no user for this member. All '-M cxl-fmw.N' options have
been parsed and saved to CXLState.fixed_windows.

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 include/hw/cxl/cxl.h | 1 -
 hw/cxl/cxl-host.c    | 1 -
 2 files changed, 2 deletions(-)

diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 75e47b6864..e3ecbef038 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -43,7 +43,6 @@ typedef struct CXLState {
     MemoryRegion host_mr;
     unsigned int next_mr_idx;
     GList *fixed_windows;
-    CXLFixedMemoryWindowOptionsList *cfmw_list;
 } CXLState;
 
 struct CXLHost {
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index c5f5fcfd64..926d3d3da7 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -335,7 +335,6 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
     for (it = cfmw_list; it; it = it->next) {
         cxl_fixed_memory_window_config(state, it->value, errp);
     }
-    state->cfmw_list = cfmw_list;
 }
 
 void cxl_machine_init(Object *obj, CXLState *state)
-- 
2.43.0
Re: [PATCH 1/3] hw/cxl: Get rid of unused cfmw_list
Posted by Zhijian Li (Fujitsu) via 2 months, 1 week ago
Jonathan,


There is a new user for cfmw_list now
https://lore.kernel.org/qemu-devel/20240704093404.1848132-1-zhao1.liu@linux.intel.com/

So I think we should drop this patch.


On 02/07/2024 22:34, Jonathan Cameron wrote:
> From: Li Zhijian <lizhijian@fujitsu.com>
> 
> There is no user for this member. All '-M cxl-fmw.N' options have
> been parsed and saved to CXLState.fixed_windows.
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   include/hw/cxl/cxl.h | 1 -
>   hw/cxl/cxl-host.c    | 1 -
>   2 files changed, 2 deletions(-)
> 
> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> index 75e47b6864..e3ecbef038 100644
> --- a/include/hw/cxl/cxl.h
> +++ b/include/hw/cxl/cxl.h
> @@ -43,7 +43,6 @@ typedef struct CXLState {
>       MemoryRegion host_mr;
>       unsigned int next_mr_idx;
>       GList *fixed_windows;
> -    CXLFixedMemoryWindowOptionsList *cfmw_list;
>   } CXLState;
>   
>   struct CXLHost {
> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> index c5f5fcfd64..926d3d3da7 100644
> --- a/hw/cxl/cxl-host.c
> +++ b/hw/cxl/cxl-host.c
> @@ -335,7 +335,6 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
>       for (it = cfmw_list; it; it = it->next) {
>           cxl_fixed_memory_window_config(state, it->value, errp);
>       }
> -    state->cfmw_list = cfmw_list;
>   }
>   
>   void cxl_machine_init(Object *obj, CXLState *state)
Re: [PATCH 1/3] hw/cxl: Get rid of unused cfmw_list
Posted by Zhao Liu 2 months, 1 week ago
On Fri, Jul 05, 2024 at 01:04:51AM +0000, Zhijian Li (Fujitsu) wrote:
> Date: Fri, 5 Jul 2024 01:04:51 +0000
> From: "Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com>
> Subject: Re: [PATCH 1/3] hw/cxl: Get rid of unused cfmw_list
> 
> Jonathan,
> 
> 
> There is a new user for cfmw_list now
> https://lore.kernel.org/qemu-devel/20240704093404.1848132-1-zhao1.liu@linux.intel.com/
> 
> So I think we should drop this patch.

Hi Zhijian,

I'm not a "real" user and that bug was originally found by code reading.

So that fix won't block your drop. :-)

And I think the fix is worth being laned before cfmw_list gets dropped,
for being able to port backwards to stable QEMU.
 
> On 02/07/2024 22:34, Jonathan Cameron wrote:
> > From: Li Zhijian <lizhijian@fujitsu.com>
> > 
> > There is no user for this member. All '-M cxl-fmw.N' options have
> > been parsed and saved to CXLState.fixed_windows.
> > 
> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >   include/hw/cxl/cxl.h | 1 -
> >   hw/cxl/cxl-host.c    | 1 -
> >   2 files changed, 2 deletions(-)
> > 
> > diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> > index 75e47b6864..e3ecbef038 100644
> > --- a/include/hw/cxl/cxl.h
> > +++ b/include/hw/cxl/cxl.h
> > @@ -43,7 +43,6 @@ typedef struct CXLState {
> >       MemoryRegion host_mr;
> >       unsigned int next_mr_idx;
> >       GList *fixed_windows;
> > -    CXLFixedMemoryWindowOptionsList *cfmw_list;
> >   } CXLState;
> >   
> >   struct CXLHost {
> > diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> > index c5f5fcfd64..926d3d3da7 100644
> > --- a/hw/cxl/cxl-host.c
> > +++ b/hw/cxl/cxl-host.c
> > @@ -335,7 +335,6 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
> >       for (it = cfmw_list; it; it = it->next) {
> >           cxl_fixed_memory_window_config(state, it->value, errp);
> >       }
> > -    state->cfmw_list = cfmw_list;
> >   }
> >   
> >   void cxl_machine_init(Object *obj, CXLState *state)
Re: [PATCH 1/3] hw/cxl: Get rid of unused cfmw_list
Posted by Zhijian Li (Fujitsu) via 2 months, 1 week ago

On 05/07/2024 10:15, Zhao Liu wrote:
>> There is a new user for cfmw_list now
>> https://lore.kernel.org/qemu-devel/20240704093404.1848132-1-zhao1.liu@linux.intel.com/
>>
>> So I think we should drop this patch.

> Hi Zhijian,
> 
> I'm not a "real" user and that bug was originally found by code reading.
> 
> So that fix won't block your drop. 🙂


"hw/cxl: Get rid of unused cfmw_list" is no longer needed.



> 
> And I think the fix is worth being laned before cfmw_list gets dropped,
> for being able to port backwards to stable QEMU.

Your fix[0] requires CXLState.cfmw_list, and I think CXLState.cfmw_list was designed for *get* purpose
but got mistake at that time.

[0] https://lore.kernel.org/qemu-devel/20240704093404.1848132-1-zhao1.liu@linux.intel.com/


>   
>> On 02/07/2024 22:34, Jonathan Cameron wrote:
>>> From: Li Zhijian<lizhijian@fujitsu.com>
>>>
>>> There is no user for this member. All '-M cxl-fmw.N' options have
>>> been parsed and saved to CXLState.fixed_windows.
>>>
>>> Signed-off-by: Li Zhijian<lizhijian@fujitsu.com>
>>> Signed-off-by: Jonathan Cameron<Jonathan.Cameron@huawei.com>
>>> ---
>>>    include/hw/cxl/cxl.h | 1 -
>>>    hw/cxl/cxl-host.c    | 1 -
>>>    2 files changed, 2 deletions(-)
>>>
>>> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
>>> index 75e47b6864..e3ecbef038 100644
>>> --- a/include/hw/cxl/cxl.h
>>> +++ b/include/hw/cxl/cxl.h
>>> @@ -43,7 +43,6 @@ typedef struct CXLState {
>>>        MemoryRegion host_mr;
>>>        unsigned int next_mr_idx;
>>>        GList *fixed_windows;
>>> -    CXLFixedMemoryWindowOptionsList *cfmw_list;
>>>    } CXLState;
>>>    
Re: [PATCH 1/3] hw/cxl: Get rid of unused cfmw_list
Posted by Jonathan Cameron via 2 months, 1 week ago
On Fri, 5 Jul 2024 02:29:34 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:

> On 05/07/2024 10:15, Zhao Liu wrote:
> >> There is a new user for cfmw_list now
> >> https://lore.kernel.org/qemu-devel/20240704093404.1848132-1-zhao1.liu@linux.intel.com/
> >>
> >> So I think we should drop this patch.  
> 
> > Hi Zhijian,
> > 
> > I'm not a "real" user and that bug was originally found by code reading.
> > 
> > So that fix won't block your drop. 🙂  
> 
> 
> "hw/cxl: Get rid of unused cfmw_list" is no longer needed.
> 
> 
> 
> > 
> > And I think the fix is worth being laned before cfmw_list gets dropped,
> > for being able to port backwards to stable QEMU.  
> 
> Your fix[0] requires CXLState.cfmw_list, and I think CXLState.cfmw_list was designed for *get* purpose
> but got mistake at that time.
> 
> [0] https://lore.kernel.org/qemu-devel/20240704093404.1848132-1-zhao1.liu@linux.intel.com/
Ok.  Michael, please ignore this one as the lack of use is result of another bug. 
I hadn't gotten to that fix yet, so missed the connection.

The rest of this series is unrelated.

Jonathan

> 
> 
> >     
> >> On 02/07/2024 22:34, Jonathan Cameron wrote:  
> >>> From: Li Zhijian<lizhijian@fujitsu.com>
> >>>
> >>> There is no user for this member. All '-M cxl-fmw.N' options have
> >>> been parsed and saved to CXLState.fixed_windows.
> >>>
> >>> Signed-off-by: Li Zhijian<lizhijian@fujitsu.com>
> >>> Signed-off-by: Jonathan Cameron<Jonathan.Cameron@huawei.com>
> >>> ---
> >>>    include/hw/cxl/cxl.h | 1 -
> >>>    hw/cxl/cxl-host.c    | 1 -
> >>>    2 files changed, 2 deletions(-)
> >>>
> >>> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> >>> index 75e47b6864..e3ecbef038 100644
> >>> --- a/include/hw/cxl/cxl.h
> >>> +++ b/include/hw/cxl/cxl.h
> >>> @@ -43,7 +43,6 @@ typedef struct CXLState {
> >>>        MemoryRegion host_mr;
> >>>        unsigned int next_mr_idx;
> >>>        GList *fixed_windows;
> >>> -    CXLFixedMemoryWindowOptionsList *cfmw_list;
> >>>    } CXLState;
> >>>