[Qemu-devel] [PATCH 0/3] hw/Kconfig: PCI & USB fixes

[Qemu-devel] [PATCH 0/3] hw/Kconfig: PCI & USB fixes

[Philippe Mathieu-Daudé]

I actually wanted to clean the USB devices (i.e. to not have
USB3 devices appear on OHCI buses) but this is too late for
the next release, so let's post the patches accumulated.

Philippe Mathieu-Daudé (3):
  hw/Kconfig: PCI bus implies PCI_DEVICES
  hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI
  hw/usb/Kconfig: USB_XHCI_NEC requires USB_XHCI

 docs/devel/kconfig.rst |  1 -
 hw/alpha/Kconfig       |  1 -
 hw/arm/Kconfig         |  4 ----
 hw/hppa/Kconfig        |  1 -
 hw/i386/Kconfig        |  1 -
 hw/pci/Kconfig         |  1 +
 hw/ppc/Kconfig         |  6 ------
 hw/riscv/Kconfig       |  1 -
 hw/sh4/Kconfig         |  1 -
 hw/sparc64/Kconfig     |  1 -
 hw/usb/Kconfig         | 11 +++++++----
 hw/usb/Makefile.objs   |  5 +++--
 12 files changed, 11 insertions(+), 23 deletions(-)

-- 
2.20.1

[Qemu-devel] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Philippe Mathieu-Daudé]

If a controller device provides a PCI bus, we can plug any PCI
daughter card on it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 docs/devel/kconfig.rst | 1 -
 hw/alpha/Kconfig       | 1 -
 hw/arm/Kconfig         | 4 ----
 hw/hppa/Kconfig        | 1 -
 hw/i386/Kconfig        | 1 -
 hw/pci/Kconfig         | 1 +
 hw/ppc/Kconfig         | 6 ------
 hw/riscv/Kconfig       | 1 -
 hw/sh4/Kconfig         | 1 -
 hw/sparc64/Kconfig     | 1 -
 10 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst
index d6f8eb0977..6c010d38d4 100644
--- a/docs/devel/kconfig.rst
+++ b/docs/devel/kconfig.rst
@@ -56,7 +56,6 @@ stanza like the following::
 
       config ARM_VIRT
          bool
-         imply PCI_DEVICES
          imply VFIO_AMD_XGBE
          imply VFIO_XGMAC
          select A15MPCORE
diff --git a/hw/alpha/Kconfig b/hw/alpha/Kconfig
index 15c59ff264..ddac1bf8e5 100644
--- a/hw/alpha/Kconfig
+++ b/hw/alpha/Kconfig
@@ -1,6 +1,5 @@
 config DP264
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     imply E1000_PCI
     select I82374
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index ab65ecd216..51db042a0a 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -1,6 +1,5 @@
 config ARM_VIRT
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     imply VFIO_AMD_XGBE
     imply VFIO_PLATFORM
@@ -159,7 +158,6 @@ config Z2
 
 config REALVIEW
     bool
-    imply PCI_DEVICES
     imply PCI_TESTDEV
     select SMC91C111
     select LAN9118
@@ -186,7 +184,6 @@ config REALVIEW
 
 config SBSA_REF
     bool
-    imply PCI_DEVICES
     select AHCI
     select ARM_SMMUV3
     select GPIO_KEY
@@ -365,7 +362,6 @@ config MPS2
 
 config FSL_IMX7
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     select A15MPCORE
     select PCI
diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index 6e5d74a825..13183ff243 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -1,6 +1,5 @@
 config DINO
     bool
-    imply PCI_DEVICES
     imply E1000_PCI
     imply VIRTIO_VGA
     select PCI
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 4ddf2a9c55..4eceab2c7c 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -10,7 +10,6 @@ config PC
     imply ISA_IPMI_BT
     imply ISA_DEBUG
     imply PARALLEL
-    imply PCI_DEVICES
     imply PVPANIC
     imply QXL
     imply SEV
diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
index 77f8b005ff..0f7267db35 100644
--- a/hw/pci/Kconfig
+++ b/hw/pci/Kconfig
@@ -1,5 +1,6 @@
 config PCI
     bool
+    imply PCI_DEVICES
 
 config PCI_EXPRESS
     bool
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index f927ec9c74..6a7f06744d 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -1,6 +1,5 @@
 config PSERIES
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     imply VIRTIO_VGA
     select DIMM
@@ -18,7 +17,6 @@ config SPAPR_RNG
 
 config POWERNV
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     select ISA_IPMI_BT
     select IPMI_LOCAL
@@ -36,7 +34,6 @@ config PPC405
 
 config PPC440
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     imply E1000_PCI
     select PCI_EXPRESS
@@ -62,7 +59,6 @@ config SAM460EX
 
 config PREP
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     select CS4231A
     select PREP_PCI
@@ -77,7 +73,6 @@ config RS6000_MC
 
 config MAC_OLDWORLD
     bool
-    imply PCI_DEVICES
     imply SUNGEM
     imply TEST_DEVICES
     select ADB
@@ -87,7 +82,6 @@ config MAC_OLDWORLD
 
 config MAC_NEWWORLD
     bool
-    imply PCI_DEVICES
     imply SUNGEM
     imply TEST_DEVICES
     select ADB
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 8674211085..636072c7f3 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -27,7 +27,6 @@ config SPIKE
 
 config RISCV_VIRT
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     select PCI
     select HART
diff --git a/hw/sh4/Kconfig b/hw/sh4/Kconfig
index 4cbce3a0ed..cfde77d04a 100644
--- a/hw/sh4/Kconfig
+++ b/hw/sh4/Kconfig
@@ -1,6 +1,5 @@
 config R2D
     bool
-    imply PCI_DEVICES
     imply TEST_DEVICES
     imply RTL8139_PCI
     select I82378 if TEST_DEVICES
diff --git a/hw/sparc64/Kconfig b/hw/sparc64/Kconfig
index d4d76a89be..125d442d65 100644
--- a/hw/sparc64/Kconfig
+++ b/hw/sparc64/Kconfig
@@ -1,6 +1,5 @@
 config SUN4U
     bool
-    imply PCI_DEVICES
     imply SUNHME
     imply TEST_DEVICES
     imply PARALLEL
-- 
2.20.1

[Qemu-devel] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[Philippe Mathieu-Daudé]

The USB_EHCI entry currently include PCI code. Since the EHCI
implementation is already split in sysbus/PCI, add a new
USB_EHCI_PCI. There are no logical changes, but the Kconfig
dependencies tree is cleaner.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/usb/Kconfig       | 9 ++++++---
 hw/usb/Makefile.objs | 5 +++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 564305e283..495c6f2d48 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -19,13 +19,16 @@ config USB_OHCI_PCI
 
 config USB_EHCI
     bool
-    default y if PCI_DEVICES
-    depends on PCI
     select USB
 
+config USB_EHCI_PCI
+    bool
+    default y if PCI_DEVICES
+    select USB_EHCI
+
 config USB_EHCI_SYSBUS
     bool
-    select USB
+    select USB_EHCI
 
 config USB_XHCI
     bool
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 81688f6e70..303ac084a0 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -6,8 +6,9 @@ common-obj-$(CONFIG_USB) += desc.o desc-msos.o
 common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o
 common-obj-$(CONFIG_USB_OHCI) += hcd-ohci.o
 common-obj-$(CONFIG_USB_OHCI_PCI) += hcd-ohci-pci.o
-common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o hcd-ehci-pci.o
-common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci.o hcd-ehci-sysbus.o
+common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o
+common-obj-$(CONFIG_USB_EHCI_PCI) += hcd-ehci-pci.o
+common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci-sysbus.o
 common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o
 common-obj-$(CONFIG_USB_XHCI_NEC) += hcd-xhci-nec.o
 common-obj-$(CONFIG_USB_MUSB) += hcd-musb.o
-- 
2.20.1

[Qemu-devel] [PATCH-for-4.1? 3/3] hw/usb/Kconfig: USB_XHCI_NEC requires USB_XHCI

[Philippe Mathieu-Daudé]

TYPE_NEC_XHCI is child of TYPE_XHCI. Add the missing Kconfig
dependency.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/usb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 495c6f2d48..555e09377b 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -40,7 +40,7 @@ config USB_XHCI_NEC
     bool
     default y if PCI_DEVICES
     depends on PCI
-    select USB
+    select USB_XHCI
 
 config USB_MUSB
     bool
-- 
2.20.1

Re: [Qemu-devel] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Thomas Huth]

On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:
> If a controller device provides a PCI bus, we can plug any PCI
> daughter card on it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  docs/devel/kconfig.rst | 1 -
>  hw/alpha/Kconfig       | 1 -
>  hw/arm/Kconfig         | 4 ----
>  hw/hppa/Kconfig        | 1 -
>  hw/i386/Kconfig        | 1 -
>  hw/pci/Kconfig         | 1 +
>  hw/ppc/Kconfig         | 6 ------
>  hw/riscv/Kconfig       | 1 -
>  hw/sh4/Kconfig         | 1 -
>  hw/sparc64/Kconfig     | 1 -
>  10 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst
> index d6f8eb0977..6c010d38d4 100644
> --- a/docs/devel/kconfig.rst
> +++ b/docs/devel/kconfig.rst
> @@ -56,7 +56,6 @@ stanza like the following::
>  
>        config ARM_VIRT
>           bool
> -         imply PCI_DEVICES
>           imply VFIO_AMD_XGBE
>           imply VFIO_XGMAC
>           select A15MPCORE
> diff --git a/hw/alpha/Kconfig b/hw/alpha/Kconfig
> index 15c59ff264..ddac1bf8e5 100644
> --- a/hw/alpha/Kconfig
> +++ b/hw/alpha/Kconfig
> @@ -1,6 +1,5 @@
>  config DP264
>      bool
> -    imply PCI_DEVICES
>      imply TEST_DEVICES
>      imply E1000_PCI
>      select I82374
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index ab65ecd216..51db042a0a 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -1,6 +1,5 @@
>  config ARM_VIRT
>      bool
> -    imply PCI_DEVICES
>      imply TEST_DEVICES
>      imply VFIO_AMD_XGBE
>      imply VFIO_PLATFORM
> @@ -159,7 +158,6 @@ config Z2
>  
>  config REALVIEW
>      bool
> -    imply PCI_DEVICES
>      imply PCI_TESTDEV
>      select SMC91C111
>      select LAN9118
> @@ -186,7 +184,6 @@ config REALVIEW
>  
>  config SBSA_REF
>      bool
> -    imply PCI_DEVICES
>      select AHCI
>      select ARM_SMMUV3
>      select GPIO_KEY
> @@ -365,7 +362,6 @@ config MPS2
>  
>  config FSL_IMX7
>      bool
> -    imply PCI_DEVICES
>      imply TEST_DEVICES
>      select A15MPCORE
>      select PCI
> diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
> index 6e5d74a825..13183ff243 100644
> --- a/hw/hppa/Kconfig
> +++ b/hw/hppa/Kconfig
> @@ -1,6 +1,5 @@
>  config DINO
>      bool
> -    imply PCI_DEVICES
>      imply E1000_PCI
>      imply VIRTIO_VGA
>      select PCI
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 4ddf2a9c55..4eceab2c7c 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -10,7 +10,6 @@ config PC
>      imply ISA_IPMI_BT
>      imply ISA_DEBUG
>      imply PARALLEL
> -    imply PCI_DEVICES
>      imply PVPANIC
>      imply QXL
>      imply SEV
> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
> index 77f8b005ff..0f7267db35 100644
> --- a/hw/pci/Kconfig
> +++ b/hw/pci/Kconfig
> @@ -1,5 +1,6 @@
>  config PCI
>      bool
> +    imply PCI_DEVICES

No, please don't change this. This was done on purpose, since almost all
PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).

 Thomas

Re: [Qemu-devel] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Peter Maydell]

On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:
>
> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:
> > If a controller device provides a PCI bus, we can plug any PCI
> > daughter card on it.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---

> > diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
> > index 77f8b005ff..0f7267db35 100644
> > --- a/hw/pci/Kconfig
> > +++ b/hw/pci/Kconfig
> > @@ -1,5 +1,6 @@
> >  config PCI
> >      bool
> > +    imply PCI_DEVICES
>
> No, please don't change this. This was done on purpose, since almost all
> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).

But that means that every board that provides PCI has to have an
"imply PCI_DEVICES" line, which is pretty clunky just to work
around an s390x limitation.

Is there some way in the Kconfig syntax for s390x to say
"no PCI_DEVICES" so we can have the corner-case be handled
by the s390x Kconfig in one place rather than in 20 places
affecting everywhere except s390x?

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Thomas Huth]

On 15/07/2019 12.19, Peter Maydell wrote:
> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:
>>> If a controller device provides a PCI bus, we can plug any PCI
>>> daughter card on it.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
> 
>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>>> index 77f8b005ff..0f7267db35 100644
>>> --- a/hw/pci/Kconfig
>>> +++ b/hw/pci/Kconfig
>>> @@ -1,5 +1,6 @@
>>>  config PCI
>>>      bool
>>> +    imply PCI_DEVICES
>>
>> No, please don't change this. This was done on purpose, since almost all
>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).
> 
> But that means that every board that provides PCI has to have an
> "imply PCI_DEVICES" line, which is pretty clunky just to work
> around an s390x limitation.
> 
> Is there some way in the Kconfig syntax for s390x to say
> "no PCI_DEVICES" so we can have the corner-case be handled
> by the s390x Kconfig in one place rather than in 20 places
> affecting everywhere except s390x?

IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
guess the correct way to fix this would be to introduce some
PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
depend on it.

 Thomas

Re: [Qemu-devel] [PATCH-for-4.1? 3/3] hw/usb/Kconfig: USB_XHCI_NEC requires USB_XHCI

[Thomas Huth]

On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:
> TYPE_NEC_XHCI is child of TYPE_XHCI. Add the missing Kconfig
> dependency.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/usb/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 495c6f2d48..555e09377b 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -40,7 +40,7 @@ config USB_XHCI_NEC
>      bool
>      default y if PCI_DEVICES
>      depends on PCI
> -    select USB
> +    select USB_XHCI
>  
>  config USB_MUSB
>      bool
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [Qemu-ppc] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[BALATON Zoltan]

On Mon, 15 Jul 2019, Philippe Mathieu-Daudé wrote:
> The USB_EHCI entry currently include PCI code. Since the EHCI
> implementation is already split in sysbus/PCI, add a new
> USB_EHCI_PCI. There are no logical changes, but the Kconfig
> dependencies tree is cleaner.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/usb/Kconfig       | 9 ++++++---
> hw/usb/Makefile.objs | 5 +++--
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 564305e283..495c6f2d48 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -19,13 +19,16 @@ config USB_OHCI_PCI
>
> config USB_EHCI
>     bool
> -    default y if PCI_DEVICES
> -    depends on PCI
>     select USB
>
> +config USB_EHCI_PCI
> +    bool
> +    default y if PCI_DEVICES
> +    select USB_EHCI
> +
> config USB_EHCI_SYSBUS
>     bool
> -    select USB
> +    select USB_EHCI

Isn't this making USB_EHCI effectively the same as USB so maybe you don't 
need to keep that around any more. Can you just add select USB to 
USB_EHCI_PCI and USB_EHCI_SYSBUS and delete USB_EHCI?

Regards,
BALATON Zoltan

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Cornelia Huck]

On Mon, 15 Jul 2019 12:48:55 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 15/07/2019 12.19, Peter Maydell wrote:
> > On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:  
> >>
> >> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:  
> >>> If a controller device provides a PCI bus, we can plug any PCI
> >>> daughter card on it.
> >>>
> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>> ---  
> >   
> >>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
> >>> index 77f8b005ff..0f7267db35 100644
> >>> --- a/hw/pci/Kconfig
> >>> +++ b/hw/pci/Kconfig
> >>> @@ -1,5 +1,6 @@
> >>>  config PCI
> >>>      bool
> >>> +    imply PCI_DEVICES  
> >>
> >> No, please don't change this. This was done on purpose, since almost all
> >> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).  
> > 
> > But that means that every board that provides PCI has to have an
> > "imply PCI_DEVICES" line, which is pretty clunky just to work
> > around an s390x limitation.
> > 
> > Is there some way in the Kconfig syntax for s390x to say
> > "no PCI_DEVICES" so we can have the corner-case be handled
> > by the s390x Kconfig in one place rather than in 20 places
> > affecting everywhere except s390x?  
> 
> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
> guess the correct way to fix this would be to introduce some
> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
> depend on it.

s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.

cc:ing Collin in case there's something else.

Re: [Qemu-devel] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[Thomas Huth]

On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:
> The USB_EHCI entry currently include PCI code. Since the EHCI
> implementation is already split in sysbus/PCI, add a new
> USB_EHCI_PCI. There are no logical changes, but the Kconfig
> dependencies tree is cleaner.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/usb/Kconfig       | 9 ++++++---
>  hw/usb/Makefile.objs | 5 +++--
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 564305e283..495c6f2d48 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -19,13 +19,16 @@ config USB_OHCI_PCI
>  
>  config USB_EHCI
>      bool
> -    default y if PCI_DEVICES
> -    depends on PCI
>      select USB
>  
> +config USB_EHCI_PCI
> +    bool
> +    default y if PCI_DEVICES
> +    select USB_EHCI
> +
>  config USB_EHCI_SYSBUS
>      bool
> -    select USB
> +    select USB_EHCI
>  
>  config USB_XHCI
>      bool
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index 81688f6e70..303ac084a0 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -6,8 +6,9 @@ common-obj-$(CONFIG_USB) += desc.o desc-msos.o
>  common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o
>  common-obj-$(CONFIG_USB_OHCI) += hcd-ohci.o
>  common-obj-$(CONFIG_USB_OHCI_PCI) += hcd-ohci-pci.o
> -common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o hcd-ehci-pci.o
> -common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci.o hcd-ehci-sysbus.o
> +common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o
> +common-obj-$(CONFIG_USB_EHCI_PCI) += hcd-ehci-pci.o
> +common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci-sysbus.o
>  common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o
>  common-obj-$(CONFIG_USB_XHCI_NEC) += hcd-xhci-nec.o
>  common-obj-$(CONFIG_USB_MUSB) += hcd-musb.o
> 

Looks cleaner this way, indeed.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [Qemu-ppc] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[Thomas Huth]

On 15/07/2019 12.54, BALATON Zoltan wrote:
> On Mon, 15 Jul 2019, Philippe Mathieu-Daudé wrote:
>> The USB_EHCI entry currently include PCI code. Since the EHCI
>> implementation is already split in sysbus/PCI, add a new
>> USB_EHCI_PCI. There are no logical changes, but the Kconfig
>> dependencies tree is cleaner.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> hw/usb/Kconfig       | 9 ++++++---
>> hw/usb/Makefile.objs | 5 +++--
>> 2 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
>> index 564305e283..495c6f2d48 100644
>> --- a/hw/usb/Kconfig
>> +++ b/hw/usb/Kconfig
>> @@ -19,13 +19,16 @@ config USB_OHCI_PCI
>>
>> config USB_EHCI
>>     bool
>> -    default y if PCI_DEVICES
>> -    depends on PCI
>>     select USB
>>
>> +config USB_EHCI_PCI
>> +    bool
>> +    default y if PCI_DEVICES
>> +    select USB_EHCI
>> +
>> config USB_EHCI_SYSBUS
>>     bool
>> -    select USB
>> +    select USB_EHCI
> 
> Isn't this making USB_EHCI effectively the same as USB so maybe you
> don't need to keep that around any more. Can you just add select USB to
> USB_EHCI_PCI and USB_EHCI_SYSBUS and delete USB_EHCI?

If you want to compile without USB_EHCI_PCI and without USB_EHCI_SYSBUS,
but with USB (e.g. due to XHCI), I think we should not include
hcd-ehci.o file in the build. So I think it's fine that we have a
separate config switch for this file.

 Thomas

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Philippe Mathieu-Daudé]

On 7/15/19 12:56 PM, Cornelia Huck wrote:
> On Mon, 15 Jul 2019 12:48:55 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 15/07/2019 12.19, Peter Maydell wrote:
>>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:  
>>>>
>>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:  
>>>>> If a controller device provides a PCI bus, we can plug any PCI
>>>>> daughter card on it.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> ---  
>>>   
>>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>>>>> index 77f8b005ff..0f7267db35 100644
>>>>> --- a/hw/pci/Kconfig
>>>>> +++ b/hw/pci/Kconfig
>>>>> @@ -1,5 +1,6 @@
>>>>>  config PCI
>>>>>      bool
>>>>> +    imply PCI_DEVICES  
>>>>
>>>> No, please don't change this. This was done on purpose, since almost all
>>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).  
>>>
>>> But that means that every board that provides PCI has to have an
>>> "imply PCI_DEVICES" line, which is pretty clunky just to work
>>> around an s390x limitation.
>>>
>>> Is there some way in the Kconfig syntax for s390x to say
>>> "no PCI_DEVICES" so we can have the corner-case be handled
>>> by the s390x Kconfig in one place rather than in 20 places
>>> affecting everywhere except s390x?  
>>
>> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
>> guess the correct way to fix this would be to introduce some
>> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
>> depend on it.
> 
> s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.

Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?

So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
legacy devices?

Other MSI compliant should select PCI_MSI instead. PCI alone seems to
give troubles.

> cc:ing Collin in case there's something else.
> 

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Cornelia Huck]

On Mon, 15 Jul 2019 13:04:28 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 7/15/19 12:56 PM, Cornelia Huck wrote:
> > On Mon, 15 Jul 2019 12:48:55 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >   
> >> On 15/07/2019 12.19, Peter Maydell wrote:  
> >>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:    
> >>>>
> >>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:    
> >>>>> If a controller device provides a PCI bus, we can plug any PCI
> >>>>> daughter card on it.
> >>>>>
> >>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>>> ---    
> >>>     
> >>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
> >>>>> index 77f8b005ff..0f7267db35 100644
> >>>>> --- a/hw/pci/Kconfig
> >>>>> +++ b/hw/pci/Kconfig
> >>>>> @@ -1,5 +1,6 @@
> >>>>>  config PCI
> >>>>>      bool
> >>>>> +    imply PCI_DEVICES    
> >>>>
> >>>> No, please don't change this. This was done on purpose, since almost all
> >>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).    
> >>>
> >>> But that means that every board that provides PCI has to have an
> >>> "imply PCI_DEVICES" line, which is pretty clunky just to work
> >>> around an s390x limitation.
> >>>
> >>> Is there some way in the Kconfig syntax for s390x to say
> >>> "no PCI_DEVICES" so we can have the corner-case be handled
> >>> by the s390x Kconfig in one place rather than in 20 places
> >>> affecting everywhere except s390x?    
> >>
> >> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
> >> guess the correct way to fix this would be to introduce some
> >> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
> >> depend on it.  
> > 
> > s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.  
> 
> Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?
> 
> So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
> legacy devices?

Wrong way around? We need MSI-X for s390x, not plain MSI or
'legacy' (whatever that is).

> 
> Other MSI compliant should select PCI_MSI instead. PCI alone seems to
> give troubles.
> 
> > cc:ing Collin in case there's something else.
> >   

Re: [Qemu-devel] [Qemu-ppc] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[BALATON Zoltan]

On Mon, 15 Jul 2019, Thomas Huth wrote:
> On 15/07/2019 12.54, BALATON Zoltan wrote:
>> On Mon, 15 Jul 2019, Philippe Mathieu-Daudé wrote:
>>> The USB_EHCI entry currently include PCI code. Since the EHCI
>>> implementation is already split in sysbus/PCI, add a new
>>> USB_EHCI_PCI. There are no logical changes, but the Kconfig
>>> dependencies tree is cleaner.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> hw/usb/Kconfig?????? | 9 ++++++---
>>> hw/usb/Makefile.objs | 5 +++--
>>> 2 files changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
>>> index 564305e283..495c6f2d48 100644
>>> --- a/hw/usb/Kconfig
>>> +++ b/hw/usb/Kconfig
>>> @@ -19,13 +19,16 @@ config USB_OHCI_PCI
>>>
>>> config USB_EHCI
>>> ??? bool
>>> -??? default y if PCI_DEVICES
>>> -??? depends on PCI
>>> ??? select USB
>>>
>>> +config USB_EHCI_PCI
>>> +??? bool
>>> +??? default y if PCI_DEVICES
>>> +??? select USB_EHCI
>>> +
>>> config USB_EHCI_SYSBUS
>>> ??? bool
>>> -??? select USB
>>> +??? select USB_EHCI
>>
>> Isn't this making USB_EHCI effectively the same as USB so maybe you
>> don't need to keep that around any more. Can you just add select USB to
>> USB_EHCI_PCI and USB_EHCI_SYSBUS and delete USB_EHCI?
>
> If you want to compile without USB_EHCI_PCI and without USB_EHCI_SYSBUS,
> but with USB (e.g. due to XHCI), I think we should not include
> hcd-ehci.o file in the build. So I think it's fine that we have a
> separate config switch for this file.

So shouldn't build depend on either USB_EHCI_PCI or USB_EHCI_SYSBUS 
selected then?

Regards,
BALATON Zoltan

Re: [Qemu-devel] [Qemu-ppc] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[Thomas Huth]

On 15/07/2019 13.10, BALATON Zoltan wrote:
> On Mon, 15 Jul 2019, Thomas Huth wrote:
>> On 15/07/2019 12.54, BALATON Zoltan wrote:
>>> On Mon, 15 Jul 2019, Philippe Mathieu-Daudé wrote:
>>>> The USB_EHCI entry currently include PCI code. Since the EHCI
>>>> implementation is already split in sysbus/PCI, add a new
>>>> USB_EHCI_PCI. There are no logical changes, but the Kconfig
>>>> dependencies tree is cleaner.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>> hw/usb/Kconfig?????? | 9 ++++++---
>>>> hw/usb/Makefile.objs | 5 +++--
>>>> 2 files changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
>>>> index 564305e283..495c6f2d48 100644
>>>> --- a/hw/usb/Kconfig
>>>> +++ b/hw/usb/Kconfig
>>>> @@ -19,13 +19,16 @@ config USB_OHCI_PCI
>>>>
>>>> config USB_EHCI
>>>> ??? bool
>>>> -??? default y if PCI_DEVICES
>>>> -??? depends on PCI
>>>> ??? select USB
>>>>
>>>> +config USB_EHCI_PCI
>>>> +??? bool
>>>> +??? default y if PCI_DEVICES
>>>> +??? select USB_EHCI
>>>> +
>>>> config USB_EHCI_SYSBUS
>>>> ??? bool
>>>> -??? select USB
>>>> +??? select USB_EHCI
>>>
>>> Isn't this making USB_EHCI effectively the same as USB so maybe you
>>> don't need to keep that around any more. Can you just add select USB to
>>> USB_EHCI_PCI and USB_EHCI_SYSBUS and delete USB_EHCI?
>>
>> If you want to compile without USB_EHCI_PCI and without USB_EHCI_SYSBUS,
>> but with USB (e.g. due to XHCI), I think we should not include
>> hcd-ehci.o file in the build. So I think it's fine that we have a
>> separate config switch for this file.
> 
> So shouldn't build depend on either USB_EHCI_PCI or USB_EHCI_SYSBUS
> selected then?

Boards that have a hard-wired sysbus EHCI device already select it, e.g.
EXYNOS4 in hw/arm/Kconfig. EHCI on a PCI card is optional, so it is only
marked with "default y if PCI_DEVICES" - which you can override in your
config if you don't need it.

 Thomas

Re: [Qemu-devel] [Qemu-ppc] [PATCH-for-4.2 2/3] hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI

[Paolo Bonzini]

On 15/07/19 13:10, BALATON Zoltan wrote:
>>
>> If you want to compile without USB_EHCI_PCI and without USB_EHCI_SYSBUS,
>> but with USB (e.g. due to XHCI), I think we should not include
>> hcd-ehci.o file in the build. So I think it's fine that we have a
>> separate config switch for this file.
> 
> So shouldn't build depend on either USB_EHCI_PCI or USB_EHCI_SYSBUS
> selected then?

No, USB_EHCI is never selected by the user.  The board or default-config
file selects either USB_EHCI_PCI or USB_EHCI_SYSBUS, which brings in
USB_EHCI, which brings in USB, which (optionally) brings in the USB devices.

Paolo

Re: [Qemu-devel] [PATCH 0/3] hw/Kconfig: PCI & USB fixes

[Paolo Bonzini]

On 15/07/19 11:55, Philippe Mathieu-Daudé wrote:
> I actually wanted to clean the USB devices (i.e. to not have
> USB3 devices appear on OHCI buses) but this is too late for
> the next release, so let's post the patches accumulated.
> 
> Philippe Mathieu-Daudé (3):
>   hw/Kconfig: PCI bus implies PCI_DEVICES
>   hw/usb/Kconfig: Add CONFIG_USB_EHCI_PCI
>   hw/usb/Kconfig: USB_XHCI_NEC requires USB_XHCI
> 
>  docs/devel/kconfig.rst |  1 -
>  hw/alpha/Kconfig       |  1 -
>  hw/arm/Kconfig         |  4 ----
>  hw/hppa/Kconfig        |  1 -
>  hw/i386/Kconfig        |  1 -
>  hw/pci/Kconfig         |  1 +
>  hw/ppc/Kconfig         |  6 ------
>  hw/riscv/Kconfig       |  1 -
>  hw/sh4/Kconfig         |  1 -
>  hw/sparc64/Kconfig     |  1 -
>  hw/usb/Kconfig         | 11 +++++++----
>  hw/usb/Makefile.objs   |  5 +++--
>  12 files changed, 11 insertions(+), 23 deletions(-)
> 

ACK for 2 and 3.  Gerd is on vacation so I'm picking them up.

Paolo

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Thomas Huth]

On 15/07/2019 13.09, Cornelia Huck wrote:
> On Mon, 15 Jul 2019 13:04:28 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 7/15/19 12:56 PM, Cornelia Huck wrote:
>>> On Mon, 15 Jul 2019 12:48:55 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>   
>>>> On 15/07/2019 12.19, Peter Maydell wrote:  
>>>>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:    
>>>>>>
>>>>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:    
>>>>>>> If a controller device provides a PCI bus, we can plug any PCI
>>>>>>> daughter card on it.
>>>>>>>
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>>> ---    
>>>>>     
>>>>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>>>>>>> index 77f8b005ff..0f7267db35 100644
>>>>>>> --- a/hw/pci/Kconfig
>>>>>>> +++ b/hw/pci/Kconfig
>>>>>>> @@ -1,5 +1,6 @@
>>>>>>>  config PCI
>>>>>>>      bool
>>>>>>> +    imply PCI_DEVICES    
>>>>>>
>>>>>> No, please don't change this. This was done on purpose, since almost all
>>>>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).    
>>>>>
>>>>> But that means that every board that provides PCI has to have an
>>>>> "imply PCI_DEVICES" line, which is pretty clunky just to work
>>>>> around an s390x limitation.
>>>>>
>>>>> Is there some way in the Kconfig syntax for s390x to say
>>>>> "no PCI_DEVICES" so we can have the corner-case be handled
>>>>> by the s390x Kconfig in one place rather than in 20 places
>>>>> affecting everywhere except s390x?    
>>>>
>>>> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
>>>> guess the correct way to fix this would be to introduce some
>>>> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
>>>> depend on it.  
>>>
>>> s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.  
>>
>> Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?
>>
>> So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
>> legacy devices?
> 
> Wrong way around? We need MSI-X for s390x, not plain MSI or
> 'legacy' (whatever that is).

With "legacy" I meant the old level-triggered interrupts from the early
PCI (non-express) days. Sorry for being imprecise here.

So maybe we need two new switches, PCI_CLASSIC (or so) and PCI_MSIX, and
then the PCI devices should be marked with "default y if PCI_CLASSIC" if
they do not have MSIX support, and with "default y if PCI_MSIX" if they
have MSI-X support?

 Thomas


 Thomas

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Philippe Mathieu-Daudé]

On 7/15/19 3:19 PM, Thomas Huth wrote:
> On 15/07/2019 13.09, Cornelia Huck wrote:
>> On Mon, 15 Jul 2019 13:04:28 +0200
>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>>> On 7/15/19 12:56 PM, Cornelia Huck wrote:
>>>> On Mon, 15 Jul 2019 12:48:55 +0200
>>>> Thomas Huth <thuth@redhat.com> wrote:
>>>>   
>>>>> On 15/07/2019 12.19, Peter Maydell wrote:  
>>>>>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:    
>>>>>>>
>>>>>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:    
>>>>>>>> If a controller device provides a PCI bus, we can plug any PCI
>>>>>>>> daughter card on it.
>>>>>>>>
>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>>>> ---    
>>>>>>     
>>>>>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>>>>>>>> index 77f8b005ff..0f7267db35 100644
>>>>>>>> --- a/hw/pci/Kconfig
>>>>>>>> +++ b/hw/pci/Kconfig
>>>>>>>> @@ -1,5 +1,6 @@
>>>>>>>>  config PCI
>>>>>>>>      bool
>>>>>>>> +    imply PCI_DEVICES    
>>>>>>>
>>>>>>> No, please don't change this. This was done on purpose, since almost all
>>>>>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).    
>>>>>>
>>>>>> But that means that every board that provides PCI has to have an
>>>>>> "imply PCI_DEVICES" line, which is pretty clunky just to work
>>>>>> around an s390x limitation.
>>>>>>
>>>>>> Is there some way in the Kconfig syntax for s390x to say
>>>>>> "no PCI_DEVICES" so we can have the corner-case be handled
>>>>>> by the s390x Kconfig in one place rather than in 20 places
>>>>>> affecting everywhere except s390x?    
>>>>>
>>>>> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
>>>>> guess the correct way to fix this would be to introduce some
>>>>> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
>>>>> depend on it.  
>>>>
>>>> s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.  
>>>
>>> Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?
>>>
>>> So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
>>> legacy devices?
>>
>> Wrong way around? We need MSI-X for s390x, not plain MSI or
>> 'legacy' (whatever that is).
> 
> With "legacy" I meant the old level-triggered interrupts from the early
> PCI (non-express) days. Sorry for being imprecise here.
> 
> So maybe we need two new switches, PCI_CLASSIC (or so) and PCI_MSIX, and
> then the PCI devices should be marked with "default y if PCI_CLASSIC" if
> they do not have MSIX support, and with "default y if PCI_MSIX" if they
> have MSI-X support?

Something like that :)

Per Wikipedia:

  Conventional PCI and PCI-X are sometimes called Parallel PCI
  in order to distinguish them technologically from their more
  recent successor PCI Express, which adopted a serial,
  lane-based architecture.

  The PCI-SIG introduced the serial PCI Express in c. 2004. At
  the same time, they renamed PCI as Conventional PCI.

  PCI Express does not have physical interrupt lines at all.
  It uses message-signaled interrupts exclusively.

What about PCI_CONVENTIONAL then?

Regards,

Phil.

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Thomas Huth]

On 15/07/2019 15.38, Philippe Mathieu-Daudé wrote:
> On 7/15/19 3:19 PM, Thomas Huth wrote:
>> On 15/07/2019 13.09, Cornelia Huck wrote:
>>> On Mon, 15 Jul 2019 13:04:28 +0200
>>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>>> On 7/15/19 12:56 PM, Cornelia Huck wrote:
>>>>> On Mon, 15 Jul 2019 12:48:55 +0200
>>>>> Thomas Huth <thuth@redhat.com> wrote:
>>>>>   
>>>>>> On 15/07/2019 12.19, Peter Maydell wrote:  
>>>>>>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:    
>>>>>>>>
>>>>>>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:    
>>>>>>>>> If a controller device provides a PCI bus, we can plug any PCI
>>>>>>>>> daughter card on it.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>>>>> ---    
>>>>>>>     
>>>>>>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>>>>>>>>> index 77f8b005ff..0f7267db35 100644
>>>>>>>>> --- a/hw/pci/Kconfig
>>>>>>>>> +++ b/hw/pci/Kconfig
>>>>>>>>> @@ -1,5 +1,6 @@
>>>>>>>>>  config PCI
>>>>>>>>>      bool
>>>>>>>>> +    imply PCI_DEVICES    
>>>>>>>>
>>>>>>>> No, please don't change this. This was done on purpose, since almost all
>>>>>>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).    
>>>>>>>
>>>>>>> But that means that every board that provides PCI has to have an
>>>>>>> "imply PCI_DEVICES" line, which is pretty clunky just to work
>>>>>>> around an s390x limitation.
>>>>>>>
>>>>>>> Is there some way in the Kconfig syntax for s390x to say
>>>>>>> "no PCI_DEVICES" so we can have the corner-case be handled
>>>>>>> by the s390x Kconfig in one place rather than in 20 places
>>>>>>> affecting everywhere except s390x?    
>>>>>>
>>>>>> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
>>>>>> guess the correct way to fix this would be to introduce some
>>>>>> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
>>>>>> depend on it.  
>>>>>
>>>>> s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.  
>>>>
>>>> Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?
>>>>
>>>> So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
>>>> legacy devices?
>>>
>>> Wrong way around? We need MSI-X for s390x, not plain MSI or
>>> 'legacy' (whatever that is).
>>
>> With "legacy" I meant the old level-triggered interrupts from the early
>> PCI (non-express) days. Sorry for being imprecise here.
>>
>> So maybe we need two new switches, PCI_CLASSIC (or so) and PCI_MSIX, and
>> then the PCI devices should be marked with "default y if PCI_CLASSIC" if
>> they do not have MSIX support, and with "default y if PCI_MSIX" if they
>> have MSI-X support?
> 
> Something like that :)
> 
> Per Wikipedia:
> 
>   Conventional PCI and PCI-X are sometimes called Parallel PCI
>   in order to distinguish them technologically from their more
>   recent successor PCI Express, which adopted a serial,
>   lane-based architecture.
> 
>   The PCI-SIG introduced the serial PCI Express in c. 2004. At
>   the same time, they renamed PCI as Conventional PCI.
> 
>   PCI Express does not have physical interrupt lines at all.
>   It uses message-signaled interrupts exclusively.
> 
> What about PCI_CONVENTIONAL then?

Sounds good to me.

 Thomas

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 7/15/19 3:19 PM, Thomas Huth wrote:
>> On 15/07/2019 13.09, Cornelia Huck wrote:
>>> On Mon, 15 Jul 2019 13:04:28 +0200
>>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>>> On 7/15/19 12:56 PM, Cornelia Huck wrote:
>>>>> On Mon, 15 Jul 2019 12:48:55 +0200
>>>>> Thomas Huth <thuth@redhat.com> wrote:
>>>>>   
>>>>>> On 15/07/2019 12.19, Peter Maydell wrote:  
>>>>>>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:    
>>>>>>>>
>>>>>>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:    
>>>>>>>>> If a controller device provides a PCI bus, we can plug any PCI
>>>>>>>>> daughter card on it.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>>>>> ---    
>>>>>>>     
>>>>>>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>>>>>>>>> index 77f8b005ff..0f7267db35 100644
>>>>>>>>> --- a/hw/pci/Kconfig
>>>>>>>>> +++ b/hw/pci/Kconfig
>>>>>>>>> @@ -1,5 +1,6 @@
>>>>>>>>>  config PCI
>>>>>>>>>      bool
>>>>>>>>> +    imply PCI_DEVICES    
>>>>>>>>
>>>>>>>> No, please don't change this. This was done on purpose, since almost all
>>>>>>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).    
>>>>>>>
>>>>>>> But that means that every board that provides PCI has to have an
>>>>>>> "imply PCI_DEVICES" line, which is pretty clunky just to work
>>>>>>> around an s390x limitation.
>>>>>>>
>>>>>>> Is there some way in the Kconfig syntax for s390x to say
>>>>>>> "no PCI_DEVICES" so we can have the corner-case be handled
>>>>>>> by the s390x Kconfig in one place rather than in 20 places
>>>>>>> affecting everywhere except s390x?    
>>>>>>
>>>>>> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
>>>>>> guess the correct way to fix this would be to introduce some
>>>>>> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
>>>>>> depend on it.  
>>>>>
>>>>> s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.  
>>>>
>>>> Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?
>>>>
>>>> So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
>>>> legacy devices?
>>>
>>> Wrong way around? We need MSI-X for s390x, not plain MSI or
>>> 'legacy' (whatever that is).
>> 
>> With "legacy" I meant the old level-triggered interrupts from the early
>> PCI (non-express) days. Sorry for being imprecise here.
>> 
>> So maybe we need two new switches, PCI_CLASSIC (or so) and PCI_MSIX, and
>> then the PCI devices should be marked with "default y if PCI_CLASSIC" if
>> they do not have MSIX support, and with "default y if PCI_MSIX" if they
>> have MSI-X support?
>
> Something like that :)
>
> Per Wikipedia:
>
>   Conventional PCI and PCI-X are sometimes called Parallel PCI
>   in order to distinguish them technologically from their more
>   recent successor PCI Express, which adopted a serial,
>   lane-based architecture.
>
>   The PCI-SIG introduced the serial PCI Express in c. 2004. At
>   the same time, they renamed PCI as Conventional PCI.
>
>   PCI Express does not have physical interrupt lines at all.
>   It uses message-signaled interrupts exclusively.
>
> What about PCI_CONVENTIONAL then?

What kinds of PCI devices are we trying to name?

Is it INTx vs. MSI vs. MSI-X?

Is it Conventional PCI vs. PCI Express?

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Cornelia Huck]

On Mon, 15 Jul 2019 18:08:45 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
> > On 7/15/19 3:19 PM, Thomas Huth wrote:  
> >> On 15/07/2019 13.09, Cornelia Huck wrote:  
> >>> On Mon, 15 Jul 2019 13:04:28 +0200
> >>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>>  
> >>>> On 7/15/19 12:56 PM, Cornelia Huck wrote:  
> >>>>> On Mon, 15 Jul 2019 12:48:55 +0200
> >>>>> Thomas Huth <thuth@redhat.com> wrote:
> >>>>>     
> >>>>>> On 15/07/2019 12.19, Peter Maydell wrote:    
> >>>>>>> On Mon, 15 Jul 2019 at 11:15, Thomas Huth <thuth@redhat.com> wrote:      
> >>>>>>>>
> >>>>>>>> On 15/07/2019 11.55, Philippe Mathieu-Daudé wrote:      
> >>>>>>>>> If a controller device provides a PCI bus, we can plug any PCI
> >>>>>>>>> daughter card on it.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>>>>>>> ---      
> >>>>>>>       
> >>>>>>>>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
> >>>>>>>>> index 77f8b005ff..0f7267db35 100644
> >>>>>>>>> --- a/hw/pci/Kconfig
> >>>>>>>>> +++ b/hw/pci/Kconfig
> >>>>>>>>> @@ -1,5 +1,6 @@
> >>>>>>>>>  config PCI
> >>>>>>>>>      bool
> >>>>>>>>> +    imply PCI_DEVICES      
> >>>>>>>>
> >>>>>>>> No, please don't change this. This was done on purpose, since almost all
> >>>>>>>> PCI_DEVICES do not work on s390x (so s390x does *not* imply PCI_DEVICES).      
> >>>>>>>
> >>>>>>> But that means that every board that provides PCI has to have an
> >>>>>>> "imply PCI_DEVICES" line, which is pretty clunky just to work
> >>>>>>> around an s390x limitation.
> >>>>>>>
> >>>>>>> Is there some way in the Kconfig syntax for s390x to say
> >>>>>>> "no PCI_DEVICES" so we can have the corner-case be handled
> >>>>>>> by the s390x Kconfig in one place rather than in 20 places
> >>>>>>> affecting everywhere except s390x?      
> >>>>>>
> >>>>>> IIRC the problem on s390x are the legacy IRQs. s390x has only MSIs. So I
> >>>>>> guess the correct way to fix this would be to introduce some
> >>>>>> PCI_LEGACY_IRQ switch and let all old devices that do not work with MSI
> >>>>>> depend on it.    
> >>>>>
> >>>>> s/MSI/MSI-X/, IIRC. Not sure how far 'legacy' would stretch.    
> >>>>
> >>>> Maybe we can have something like PCI_LEGACY_DEVICES and PCI_MSI_DEVICES?
> >>>>
> >>>> So if s390x only selects PCI_LEGACY (not PCI_MSI) bus, then it only get
> >>>> legacy devices?  
> >>>
> >>> Wrong way around? We need MSI-X for s390x, not plain MSI or
> >>> 'legacy' (whatever that is).  
> >> 
> >> With "legacy" I meant the old level-triggered interrupts from the early
> >> PCI (non-express) days. Sorry for being imprecise here.
> >> 
> >> So maybe we need two new switches, PCI_CLASSIC (or so) and PCI_MSIX, and
> >> then the PCI devices should be marked with "default y if PCI_CLASSIC" if
> >> they do not have MSIX support, and with "default y if PCI_MSIX" if they
> >> have MSI-X support?  
> >
> > Something like that :)
> >
> > Per Wikipedia:
> >
> >   Conventional PCI and PCI-X are sometimes called Parallel PCI
> >   in order to distinguish them technologically from their more
> >   recent successor PCI Express, which adopted a serial,
> >   lane-based architecture.
> >
> >   The PCI-SIG introduced the serial PCI Express in c. 2004. At
> >   the same time, they renamed PCI as Conventional PCI.
> >
> >   PCI Express does not have physical interrupt lines at all.
> >   It uses message-signaled interrupts exclusively.
> >
> > What about PCI_CONVENTIONAL then?  
> 
> What kinds of PCI devices are we trying to name?
> 
> Is it INTx vs. MSI vs. MSI-X?

I think for s390x we need (INTx || MSI) vs MSI-X...

> 
> Is it Conventional PCI vs. PCI Express?

...while this is probably more INTx vs (MSI || MSI-X)?

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Paolo Bonzini]

On 15/07/19 18:12, Cornelia Huck wrote:
>> Is it INTx vs. MSI vs. MSI-X?
> 
> I think for s390x we need (INTx || MSI) vs MSI-X...

I think MSI vs MSI-X is just how it's configured, not the actual
behavior, so it should be irrelevant.

Paolo

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/07/19 18:12, Cornelia Huck wrote:
>>> Is it INTx vs. MSI vs. MSI-X?
>> 
>> I think for s390x we need (INTx || MSI) vs MSI-X...
>
> I think MSI vs MSI-X is just how it's configured, not the actual
> behavior, so it should be irrelevant.

My best guesses from confusing information, please correct
misunderstandings:

1. PCI devices need to support MSI or MSI-X to work with s390x.

2. We want a way to Kconfig the PCI devices that work, i.e. only the
   ones that support MSI or MSI-X.

I don't like calling devices without any MSI capability "classic" or
"conventional devices".  I'd rather call them "MSI-capable" or
"MSI/MSI-X devices".

Since PCI-E devices must implement MSI or MSI-X, we could perhaps
configure just the PCI-E devices for s390x.  No need to invent a new
name then.  Do we even have device models that can do MSI but not E?

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Thomas Huth]

On 16/07/2019 15.06, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 15/07/19 18:12, Cornelia Huck wrote:
>>>> Is it INTx vs. MSI vs. MSI-X?
>>>
>>> I think for s390x we need (INTx || MSI) vs MSI-X...
>>
>> I think MSI vs MSI-X is just how it's configured, not the actual
>> behavior, so it should be irrelevant.
> 
> My best guesses from confusing information, please correct
> misunderstandings:
> 
> 1. PCI devices need to support MSI or MSI-X to work with s390x.

I think Cornelia mentioned that we might even require MSI-X there ...
Collin (being listed as "S390 PCI" maintainer), can you confirm?

> 2. We want a way to Kconfig the PCI devices that work, i.e. only the
>    ones that support MSI or MSI-X.
> 
> I don't like calling devices without any MSI capability "classic" or
> "conventional devices".  I'd rather call them "MSI-capable" or
> "MSI/MSI-X devices".

But you still need a config switch that you can set to enable the
non-MSI devices ... what's so wrong with "conventional" if that's even
the wording from the PCI SIG?

> Since PCI-E devices must implement MSI or MSI-X, we could perhaps
> configure just the PCI-E devices for s390x.  No need to invent a new
> name then.  Do we even have device models that can do MSI but not E?

I don't know if anybody ever tried a non-virtio PCIe device from QEMU on
s390x ... I guess there will be quite a bit of other hurdles, too (e.g.
the Linux driver also has to work without MMIO), so I'm not sure whether
we really want to enable all PCIe devices there...

 Thomas

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Collin Walling]

On 7/16/19 11:04 AM, Thomas Huth wrote:
> On 16/07/2019 15.06, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> On 15/07/19 18:12, Cornelia Huck wrote:
>>>>> Is it INTx vs. MSI vs. MSI-X?
>>>>
>>>> I think for s390x we need (INTx || MSI) vs MSI-X...
>>>
>>> I think MSI vs MSI-X is just how it's configured, not the actual
>>> behavior, so it should be irrelevant.
>>
>> My best guesses from confusing information, please correct
>> misunderstandings:
>>
>> 1. PCI devices need to support MSI or MSI-X to work with s390x.
> 
> I think Cornelia mentioned that we might even require MSI-X there ...
> Collin (being listed as "S390 PCI" maintainer), can you confirm?
> 

MSI-X is mandatory for PCI devices in the s390 architecture.

>> 2. We want a way to Kconfig the PCI devices that work, i.e. only the
>>     ones that support MSI or MSI-X.
>>
>> I don't like calling devices without any MSI capability "classic" or
>> "conventional devices".  I'd rather call them "MSI-capable" or
>> "MSI/MSI-X devices".
> 
> But you still need a config switch that you can set to enable the
> non-MSI devices ... what's so wrong with "conventional" if that's even
> the wording from the PCI SIG?
> 
>> Since PCI-E devices must implement MSI or MSI-X, we could perhaps
>> configure just the PCI-E devices for s390x.  No need to invent a new
>> name then.  Do we even have device models that can do MSI but not E?
> 
> I don't know if anybody ever tried a non-virtio PCIe device from QEMU on
> s390x ... I guess there will be quite a bit of other hurdles, too (e.g.
> the Linux driver also has to work without MMIO), so I'm not sure whether
> we really want to enable all PCIe devices there...
> 
>   Thomas
> 

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Paolo Bonzini]

On 17/07/19 14:59, Collin Walling wrote:
> On 7/16/19 11:04 AM, Thomas Huth wrote:
>> On 16/07/2019 15.06, Markus Armbruster wrote:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>
>>>> On 15/07/19 18:12, Cornelia Huck wrote:
>>>>>> Is it INTx vs. MSI vs. MSI-X?
>>>>>
>>>>> I think for s390x we need (INTx || MSI) vs MSI-X...
>>>>
>>>> I think MSI vs MSI-X is just how it's configured, not the actual
>>>> behavior, so it should be irrelevant.
>>>
>>> My best guesses from confusing information, please correct
>>> misunderstandings:
>>>
>>> 1. PCI devices need to support MSI or MSI-X to work with s390x.
>>
>> I think Cornelia mentioned that we might even require MSI-X there ...
>> Collin (being listed as "S390 PCI" maintainer), can you confirm?
> 
> MSI-X is mandatory for PCI devices in the s390 architecture.

How are MSI and MSI-X distinguished?

Paolo

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Collin Walling]

On 7/17/19 9:52 AM, Paolo Bonzini wrote:
> On 17/07/19 14:59, Collin Walling wrote:
>> On 7/16/19 11:04 AM, Thomas Huth wrote:
>>> On 16/07/2019 15.06, Markus Armbruster wrote:
>>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>>
>>>>> On 15/07/19 18:12, Cornelia Huck wrote:
>>>>>>> Is it INTx vs. MSI vs. MSI-X?
>>>>>>
>>>>>> I think for s390x we need (INTx || MSI) vs MSI-X...
>>>>>
>>>>> I think MSI vs MSI-X is just how it's configured, not the actual
>>>>> behavior, so it should be irrelevant.
>>>>
>>>> My best guesses from confusing information, please correct
>>>> misunderstandings:
>>>>
>>>> 1. PCI devices need to support MSI or MSI-X to work with s390x.
>>>
>>> I think Cornelia mentioned that we might even require MSI-X there ...
>>> Collin (being listed as "S390 PCI" maintainer), can you confirm?
>>
>> MSI-X is mandatory for PCI devices in the s390 architecture.
> 
> How are MSI and MSI-X distinguished?
> 
> Paolo
> 

PCI host plugging will check for the MSI-X capability on the
PCI device. If the MSI-X cap is missing, we fail device plugging.
We do not check for MSI. Only MSI-X.

Specifically, the capability is represented by PCI_CAP_ID_MSIX
in pci_regs.h

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Paolo Bonzini]

On 17/07/19 16:54, Collin Walling wrote:
> PCI host plugging will check for the MSI-X capability on the
> PCI device. If the MSI-X cap is missing, we fail device plugging.
> We do not check for MSI. Only MSI-X.
> 
> Specifically, the capability is represented by PCI_CAP_ID_MSIX
> in pci_regs.h

The code in Linux says that single MSIs are supported too:

       if (type == PCI_CAP_ID_MSI && nvec > 1)
               return 1;

Paolo

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Cornelia Huck]

On Wed, 17 Jul 2019 17:04:54 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 17/07/19 16:54, Collin Walling wrote:
> > PCI host plugging will check for the MSI-X capability on the
> > PCI device. If the MSI-X cap is missing, we fail device plugging.
> > We do not check for MSI. Only MSI-X.
> > 
> > Specifically, the capability is represented by PCI_CAP_ID_MSIX
> > in pci_regs.h  
> 
> The code in Linux says that single MSIs are supported too:
> 
>        if (type == PCI_CAP_ID_MSI && nvec > 1)
>                return 1;

Interestingly, the check for MSI-X in QEMU seems to have been
introduced in 857cc71985dc ("s390x/pci: merge msix init functions"),
but that commit does not give a rationale (maybe it just referred to the
existing code structure?)

A quick look through the code suggests that single MSIs should be
supportable; can someone with access to the details verify?

Re: [Qemu-devel] [qemu-s390x] [RFC PATCH 1/3] hw/Kconfig: PCI bus implies PCI_DEVICES

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 16/07/2019 15.06, Markus Armbruster wrote:
[...]
>> 2. We want a way to Kconfig the PCI devices that work, i.e. only the
>>    ones that support MSI or MSI-X.
>> 
>> I don't like calling devices without any MSI capability "classic" or
>> "conventional devices".  I'd rather call them "MSI-capable" or
>> "MSI/MSI-X devices".
>
> But you still need a config switch that you can set to enable the
> non-MSI devices ... what's so wrong with "conventional" if that's even
> the wording from the PCI SIG?

PCI Express Base Specification, rev. 3.0, section Terms and
Acronyms:

    conventional PCI    Behavior or features that conform to the PCI
                        Local Bus Specification.

In other words, "conventional PCI" is non-Express PCI.

A PCI Express device *must* support MSI or MSI-X.  See PCI Express Base
Specification, rev. 3.0, section 7.7. MSI and MSI-X Capability
Structures.

A "conventional" PCI device *may* support MSI or MSI-X.  See PCI Local
Bus Specification Revision 3.0 section 6.8.  Message Signaled
Interrupts.

Conflating MSI-capable with conventional vs. Express is therefore wrong.

[...]