[PATCH 6/5] include/hw/cxl: Break inclusion loop

Markus Armbruster posted 5 patches 3 years, 2 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>, Richard Henderson <richard.henderson@linaro.org>, Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, BALATON Zoltan <balaton@eik.bme.hu>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, John Snow <jsnow@redhat.com>, Alberto Garcia <berto@igalia.com>, Corey Minyard <minyard@acm.org>, "Hervé Poussineau" <hpoussin@reactos.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Vikram Garhwal <fnu.vikram@xilinx.com>, Jason Wang <jasowang@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Stefan Weil <sw@weilnetz.de>, Jiri Pirko <jiri@resnulli.us>, Sven Schnelle <svens@stackframe.org>, Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Helge Deller <deller@gmx.de>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Yoshinori Sato <ysato@users.sourceforge.jp>, Magnus Damm <magnus.damm@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Yuval Shaia <yuval.shaia.ml@gmail.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Fam Zheng <fam@euphon.net>, Alex Williamson <alex.williamson@redhat.com>, Beniamino Galvani <b.galvani@gmail.com>, Ben Widawsky <ben.widawsky@intel.com>, Jonathan Cameron <jonathan.cameron@huawei.com>, Andrey Smirnov <andrew.smirnov@gmail.com>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Song Gao <gaosong@loongson.cn>, Paul Burton <paulburton@kernel.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, John G Johnson <john.g.johnson@oracle.com>, Bin Meng <bin.meng@windriver.com>
There is a newer version of this series
[PATCH 6/5] include/hw/cxl: Break inclusion loop
Posted by Markus Armbruster 3 years, 2 months ago
hw/cxl/cxl_pci.h and hw/cxl/cxl_cdat.h include each other.  Neither
header actually needs the other one.  Drop both #include directives.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/cxl/cxl_cdat.h | 1 -
 include/hw/cxl/cxl_pci.h  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
index 7f67638685..e3fd737f9d 100644
--- a/include/hw/cxl/cxl_cdat.h
+++ b/include/hw/cxl/cxl_cdat.h
@@ -10,7 +10,6 @@
 #ifndef CXL_CDAT_H
 #define CXL_CDAT_H
 
-#include "hw/cxl/cxl_pci.h"
 #include "hw/pci/pcie_doe.h"
 
 /*
diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
index aca14845ab..01e15ed5b4 100644
--- a/include/hw/cxl/cxl_pci.h
+++ b/include/hw/cxl/cxl_pci.h
@@ -11,7 +11,6 @@
 #define CXL_PCI_H
 
 #include "qemu/compiler.h"
-#include "hw/cxl/cxl_cdat.h"
 
 #define CXL_VENDOR_ID 0x1e98
 
-- 
2.37.3
Re: [PATCH 6/5] include/hw/cxl: Break inclusion loop
Posted by Markus Armbruster 3 years, 2 months ago
Markus Armbruster <armbru@redhat.com> writes:

> hw/cxl/cxl_pci.h and hw/cxl/cxl_cdat.h include each other.  Neither
> header actually needs the other one.  Drop both #include directives.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/hw/cxl/cxl_cdat.h | 1 -
>  include/hw/cxl/cxl_pci.h  | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
> index 7f67638685..e3fd737f9d 100644
> --- a/include/hw/cxl/cxl_cdat.h
> +++ b/include/hw/cxl/cxl_cdat.h
> @@ -10,7 +10,6 @@
>  #ifndef CXL_CDAT_H
>  #define CXL_CDAT_H
>  
> -#include "hw/cxl/cxl_pci.h"
>  #include "hw/pci/pcie_doe.h"
>  
>  /*
> diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
> index aca14845ab..01e15ed5b4 100644
> --- a/include/hw/cxl/cxl_pci.h
> +++ b/include/hw/cxl/cxl_pci.h
> @@ -11,7 +11,6 @@
>  #define CXL_PCI_H
>  
>  #include "qemu/compiler.h"
> -#include "hw/cxl/cxl_cdat.h"
>  
>  #define CXL_VENDOR_ID 0x1e98

Friday afternoon post with insufficient testing...  Everything still
builds fine, but cxl_component.h is no longer self-contained.  I'll
squash in the appended patch and revise the commit message.


diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index 5dca21e95b..78f83ed742 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -19,6 +19,7 @@
 #include "qemu/range.h"
 #include "qemu/typedefs.h"
 #include "hw/cxl/cxl_cdat.h"
+#include "hw/cxl/cxl_pci.h"
 #include "hw/register.h"
 #include "qapi/error.h"
Re: [PATCH 6/5] include/hw/cxl: Break inclusion loop
Posted by Jonathan Cameron via 3 years, 1 month ago
On Sat, 10 Dec 2022 08:09:06 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Markus Armbruster <armbru@redhat.com> writes:
> 
> > hw/cxl/cxl_pci.h and hw/cxl/cxl_cdat.h include each other.  Neither
> > header actually needs the other one.  Drop both #include directives.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  include/hw/cxl/cxl_cdat.h | 1 -
> >  include/hw/cxl/cxl_pci.h  | 1 -
> >  2 files changed, 2 deletions(-)
> >
> > diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
> > index 7f67638685..e3fd737f9d 100644
> > --- a/include/hw/cxl/cxl_cdat.h
> > +++ b/include/hw/cxl/cxl_cdat.h
> > @@ -10,7 +10,6 @@
> >  #ifndef CXL_CDAT_H
> >  #define CXL_CDAT_H
> >  
> > -#include "hw/cxl/cxl_pci.h"
> >  #include "hw/pci/pcie_doe.h"

The include was to get to CXL_VENDOR_ID which is in hw/cxl/cxl_pci.h
Can move that elsewhere perhaps, though I don't think we need to
if we break the loop by dropping the other one.


> >  
> >  /*
> > diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
> > index aca14845ab..01e15ed5b4 100644
> > --- a/include/hw/cxl/cxl_pci.h
> > +++ b/include/hw/cxl/cxl_pci.h
> > @@ -11,7 +11,6 @@
> >  #define CXL_PCI_H
> >  
> >  #include "qemu/compiler.h"
> > -#include "hw/cxl/cxl_cdat.h"
Guess that's a left over of some earlier refactoring. Good to get rid
of this one.

> >  
> >  #define CXL_VENDOR_ID 0x1e98  
> 
> Friday afternoon post with insufficient testing...  Everything still
> builds fine, but cxl_component.h is no longer self-contained.  I'll
> squash in the appended patch and revise the commit message.

By staring at the code rather than any automation I'm failing to spot
what it needs from cxl_pci.h.  Can you add that info to the commit message?

> 
> 
> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> index 5dca21e95b..78f83ed742 100644
> --- a/include/hw/cxl/cxl_component.h
> +++ b/include/hw/cxl/cxl_component.h
> @@ -19,6 +19,7 @@
>  #include "qemu/range.h"
>  #include "qemu/typedefs.h"
>  #include "hw/cxl/cxl_cdat.h"
> +#include "hw/cxl/cxl_pci.h"
>  #include "hw/register.h"
>  #include "qapi/error.h"
>  
>
Re: [PATCH 6/5] include/hw/cxl: Break inclusion loop
Posted by Markus Armbruster 3 years, 1 month ago
Jonathan Cameron <Jonathan.Cameron@Huawei.com> writes:

> On Sat, 10 Dec 2022 08:09:06 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>> > hw/cxl/cxl_pci.h and hw/cxl/cxl_cdat.h include each other.  Neither
>> > header actually needs the other one.  Drop both #include directives.
>> >
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> > ---
>> >  include/hw/cxl/cxl_cdat.h | 1 -
>> >  include/hw/cxl/cxl_pci.h  | 1 -
>> >  2 files changed, 2 deletions(-)
>> >
>> > diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
>> > index 7f67638685..e3fd737f9d 100644
>> > --- a/include/hw/cxl/cxl_cdat.h
>> > +++ b/include/hw/cxl/cxl_cdat.h
>> > @@ -10,7 +10,6 @@
>> >  #ifndef CXL_CDAT_H
>> >  #define CXL_CDAT_H
>> >  
>> > -#include "hw/cxl/cxl_pci.h"
>> >  #include "hw/pci/pcie_doe.h"
>
> The include was to get to CXL_VENDOR_ID which is in hw/cxl/cxl_pci.h
> Can move that elsewhere perhaps, though I don't think we need to
> if we break the loop by dropping the other one.

It's used only in a macro.  If you use the macro, you need to include
cxl_pci.h.

Would you like me to keep this #include?

>> >  /*
>> > diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
>> > index aca14845ab..01e15ed5b4 100644
>> > --- a/include/hw/cxl/cxl_pci.h
>> > +++ b/include/hw/cxl/cxl_pci.h
>> > @@ -11,7 +11,6 @@
>> >  #define CXL_PCI_H
>> >  
>> >  #include "qemu/compiler.h"
>> > -#include "hw/cxl/cxl_cdat.h"
> Guess that's a left over of some earlier refactoring. Good to get rid
> of this one.
>
>> >  
>> >  #define CXL_VENDOR_ID 0x1e98  
>> 
>> Friday afternoon post with insufficient testing...  Everything still
>> builds fine, but cxl_component.h is no longer self-contained.  I'll
>> squash in the appended patch and revise the commit message.
>
> By staring at the code rather than any automation I'm failing to spot
> what it needs from cxl_pci.h.  Can you add that info to the commit message?

It's CXL20_MAX_DVSEC.

>> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
>> index 5dca21e95b..78f83ed742 100644
>> --- a/include/hw/cxl/cxl_component.h
>> +++ b/include/hw/cxl/cxl_component.h
>> @@ -19,6 +19,7 @@
>>  #include "qemu/range.h"
>>  #include "qemu/typedefs.h"
>>  #include "hw/cxl/cxl_cdat.h"
>> +#include "hw/cxl/cxl_pci.h"
>>  #include "hw/register.h"
>>  #include "qapi/error.h"
>>  
>>
Re: [PATCH 6/5] include/hw/cxl: Break inclusion loop
Posted by Jonathan Cameron via 3 years, 1 month ago
On Thu, 15 Dec 2022 08:34:10 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Jonathan Cameron <Jonathan.Cameron@Huawei.com> writes:
> 
> > On Sat, 10 Dec 2022 08:09:06 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >  
> >> Markus Armbruster <armbru@redhat.com> writes:
> >>   
> >> > hw/cxl/cxl_pci.h and hw/cxl/cxl_cdat.h include each other.  Neither
> >> > header actually needs the other one.  Drop both #include directives.
> >> >
> >> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> > ---
> >> >  include/hw/cxl/cxl_cdat.h | 1 -
> >> >  include/hw/cxl/cxl_pci.h  | 1 -
> >> >  2 files changed, 2 deletions(-)
> >> >
> >> > diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
> >> > index 7f67638685..e3fd737f9d 100644
> >> > --- a/include/hw/cxl/cxl_cdat.h
> >> > +++ b/include/hw/cxl/cxl_cdat.h
> >> > @@ -10,7 +10,6 @@
> >> >  #ifndef CXL_CDAT_H
> >> >  #define CXL_CDAT_H
> >> >  
> >> > -#include "hw/cxl/cxl_pci.h"
> >> >  #include "hw/pci/pcie_doe.h"  
> >
> > The include was to get to CXL_VENDOR_ID which is in hw/cxl/cxl_pci.h
> > Can move that elsewhere perhaps, though I don't think we need to
> > if we break the loop by dropping the other one.  
> 
> It's used only in a macro.  If you use the macro, you need to include
> cxl_pci.h.
> 
> Would you like me to keep this #include?

yes. That would be my preference.

> 
> >> >  /*
> >> > diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
> >> > index aca14845ab..01e15ed5b4 100644
> >> > --- a/include/hw/cxl/cxl_pci.h
> >> > +++ b/include/hw/cxl/cxl_pci.h
> >> > @@ -11,7 +11,6 @@
> >> >  #define CXL_PCI_H
> >> >  
> >> >  #include "qemu/compiler.h"
> >> > -#include "hw/cxl/cxl_cdat.h"  
> > Guess that's a left over of some earlier refactoring. Good to get rid
> > of this one.
> >  
> >> >  
> >> >  #define CXL_VENDOR_ID 0x1e98    
> >> 
> >> Friday afternoon post with insufficient testing...  Everything still
> >> builds fine, but cxl_component.h is no longer self-contained.  I'll
> >> squash in the appended patch and revise the commit message.  
> >
> > By staring at the code rather than any automation I'm failing to spot
> > what it needs from cxl_pci.h.  Can you add that info to the commit message?  
> 
> It's CXL20_MAX_DVSEC.
ah. Make sense. Thanks.
> 
> >> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> >> index 5dca21e95b..78f83ed742 100644
> >> --- a/include/hw/cxl/cxl_component.h
> >> +++ b/include/hw/cxl/cxl_component.h
> >> @@ -19,6 +19,7 @@
> >>  #include "qemu/range.h"
> >>  #include "qemu/typedefs.h"
> >>  #include "hw/cxl/cxl_cdat.h"
> >> +#include "hw/cxl/cxl_pci.h"
> >>  #include "hw/register.h"
> >>  #include "qapi/error.h"
> >>  
> >>   
>