[Qemu-devel] [for-4.0 PATCH v3 0/9] pcie: Enhanced link speed and width support

[Qemu-devel] [for-4.0 PATCH v3 0/9] pcie: Enhanced link speed and width support

[Alex Williamson]

v2->v3:
 - Michael suggested offline that we not commit the pcie-root-port
   driver API to support arbitrary speeds and widths without some
   necessary use case where it's required to set these outside of
   the machine type defaults.  These options therefore become
   experimental, x-speed and x-width, with the expectation that users
   can either update their machine type or use experimental options
   for old machine types.  Patches 6 & 9 affected.  Leaving Geoffrey's
   Tested-by on patch 6 as this is a superficial change.
 - Rolled in David's Ack for spapr 4.0 machine type (patch 8).

v1->v2:
 - Update for QEMU release numbering, next is 4.0 not 3.2.  Only
   patch 8 and the commit log of patch 9 updated.

RFC->v1:
 - Add Cc reported by get_maintainer
 - Fixup some commit logs (no code changes in patches 1-7)
 - Add Geoffrey's Tested-by
 - Add patches 8 & 9 which define a QEMU 3.2 machine type and cranking
   up the link speed and width for that machine type while maintaining
   compatibile speeds for older machine types (testing requested for
   non-x86 machine types)
 - Various other users have also reported success with this series
   (/r/VFIO)

Original cover letter:

QEMU exposes gen1 PCI-express interconnect devices supporting only
2.5GT/s and x1 width.  It might not seem obvious that a virtual
bandwidth limitation can result in a real performance degradation, but
it's been reported that in some configurations assigned GPUs might not
scale their link speed up to the maximum supported value if the
downstream port above it only advertises limited link support.

As proposed[1] this series effectively implements virtual link
negotiation on downstream ports and enhances the generic PCIe root
port to allow user configurable speeds and widths.  The "negotiation"
simply mirrors the link status of the connected downstream device
providing the appearance of dynamic link speed scaling to match the
endpoint device.  Not yet implemented from the proposal is support
for globally updating defaults based on machine type, though the
foundation is provided here by allowing supporting PCIESlots to
implement an instance_init callback which can call into a common
helper for this.

I have not specifically tested migration with this, but we already
consider LNKSTA to be dynamic and the other changes implemented here
are static config space changes with no changes being implemented for
devices using default values, ie. they should be compatible by virtue
of existing config space migration support.

I think I've covered the required link related registers to support
PCIe 4.0, but please let me know if I've missed any.

Testing and feedback appreciated, patch 6/7 provides example qemu:arg
options and requirements to use with existing libvirt.  Native libvirt
support TBD.  Thanks,

Alex

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03086.html
---

Alex Williamson (9):
      pcie: Create enums for link speed and width
      pci: Sync PCIe downstream port LNKSTA on read
      qapi: Define PCIe link speed and width properties
      pcie: Add link speed and width fields to PCIESlot
      pcie: Fill PCIESlot link fields to support higher speeds and widths
      pcie: Allow generic PCIe root port to specify link speed and width
      vfio/pci: Remove PCIe Link Status emulation
      q35/440fx/arm/spapr: Add QEMU 4.0 machine type
      pcie: Fast PCIe root ports for new machines


 hw/arm/virt.c                      |   19 +++-
 hw/core/qdev-properties.c          |  178 ++++++++++++++++++++++++++++++++++++
 hw/i386/pc_piix.c                  |   15 ++-
 hw/i386/pc_q35.c                   |   13 ++-
 hw/pci-bridge/gen_pcie_root_port.c |    4 +
 hw/pci-bridge/pcie_root_port.c     |   14 +++
 hw/pci/pci.c                       |    4 +
 hw/pci/pcie.c                      |  118 +++++++++++++++++++++++-
 hw/ppc/spapr.c                     |   25 ++++-
 hw/vfio/pci.c                      |    9 --
 include/hw/compat.h                |   11 ++
 include/hw/i386/pc.h               |    3 +
 include/hw/pci/pci.h               |   13 +++
 include/hw/pci/pcie.h              |    1 
 include/hw/pci/pcie_port.h         |    4 +
 include/hw/pci/pcie_regs.h         |   23 ++++-
 include/hw/qdev-properties.h       |    8 ++
 qapi/common.json                   |   42 ++++++++
 18 files changed, 482 insertions(+), 22 deletions(-)

[Qemu-devel] [for-4.0 PATCH v3 1/9] pcie: Create enums for link speed and width

[Alex Williamson]

In preparation for reporting higher virtual link speeds and widths,
create enums and macros to help us manage them.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci/pcie.c              |    7 ++++---
 hw/vfio/pci.c              |    3 ++-
 include/hw/pci/pcie_regs.h |   23 +++++++++++++++++++++--
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6c91bd44a0a5..914a5261a79b 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -68,11 +68,12 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
     pci_set_long(exp_cap + PCI_EXP_LNKCAP,
                  (port << PCI_EXP_LNKCAP_PN_SHIFT) |
                  PCI_EXP_LNKCAP_ASPMS_0S |
-                 PCI_EXP_LNK_MLW_1 |
-                 PCI_EXP_LNK_LS_25);
+                 QEMU_PCI_EXP_LNKCAP_MLW(QEMU_PCI_EXP_LNK_X1) |
+                 QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT));
 
     pci_set_word(exp_cap + PCI_EXP_LNKSTA,
-                 PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
+                 QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1) |
+                 QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT));
 
     if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
         pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5c7bd9698496..74f9a46b4be0 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1897,7 +1897,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
                                    PCI_EXP_TYPE_ENDPOINT << 4,
                                    PCI_EXP_FLAGS_TYPE);
             vfio_add_emulated_long(vdev, pos + PCI_EXP_LNKCAP,
-                                   PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25, ~0);
+                           QEMU_PCI_EXP_LNKCAP_MLW(QEMU_PCI_EXP_LNK_X1) |
+                           QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT), ~0);
             vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKCTL, 0, ~0);
         }
 
diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
index a95522a13b04..ad4e7808b8ac 100644
--- a/include/hw/pci/pcie_regs.h
+++ b/include/hw/pci/pcie_regs.h
@@ -34,10 +34,29 @@
 
 /* PCI_EXP_LINK{CAP, STA} */
 /* link speed */
-#define PCI_EXP_LNK_LS_25               1
+typedef enum PCIExpLinkSpeed {
+    QEMU_PCI_EXP_LNK_2_5GT = 1,
+    QEMU_PCI_EXP_LNK_5GT,
+    QEMU_PCI_EXP_LNK_8GT,
+    QEMU_PCI_EXP_LNK_16GT,
+} PCIExpLinkSpeed;
+
+#define QEMU_PCI_EXP_LNKCAP_MLS(speed)  (speed)
+#define QEMU_PCI_EXP_LNKSTA_CLS         QEMU_PCI_EXP_LNKCAP_MLS
+
+typedef enum PCIExpLinkWidth {
+    QEMU_PCI_EXP_LNK_X1 = 1,
+    QEMU_PCI_EXP_LNK_X2 = 2,
+    QEMU_PCI_EXP_LNK_X4 = 4,
+    QEMU_PCI_EXP_LNK_X8 = 8,
+    QEMU_PCI_EXP_LNK_X12 = 12,
+    QEMU_PCI_EXP_LNK_X16 = 16,
+    QEMU_PCI_EXP_LNK_X32 = 32,
+} PCIExpLinkWidth;
 
 #define PCI_EXP_LNK_MLW_SHIFT           ctz32(PCI_EXP_LNKCAP_MLW)
-#define PCI_EXP_LNK_MLW_1               (1 << PCI_EXP_LNK_MLW_SHIFT)
+#define QEMU_PCI_EXP_LNKCAP_MLW(width)  (width << PCI_EXP_LNK_MLW_SHIFT)
+#define QEMU_PCI_EXP_LNKSTA_NLW         QEMU_PCI_EXP_LNKCAP_MLW
 
 /* PCI_EXP_LINKCAP */
 #define PCI_EXP_LNKCAP_ASPMS_SHIFT      ctz32(PCI_EXP_LNKCAP_ASPMS)

Re: [Qemu-devel] [for-4.0 PATCH v3 1/9] pcie: Create enums for link speed and width

[Philippe Mathieu-Daudé]

On 4/12/18 17:26, Alex Williamson wrote:
> In preparation for reporting higher virtual link speeds and widths,
> create enums and macros to help us manage them.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

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

> ---
>  hw/pci/pcie.c              |    7 ++++---
>  hw/vfio/pci.c              |    3 ++-
>  include/hw/pci/pcie_regs.h |   23 +++++++++++++++++++++--
>  3 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 6c91bd44a0a5..914a5261a79b 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -68,11 +68,12 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
>      pci_set_long(exp_cap + PCI_EXP_LNKCAP,
>                   (port << PCI_EXP_LNKCAP_PN_SHIFT) |
>                   PCI_EXP_LNKCAP_ASPMS_0S |
> -                 PCI_EXP_LNK_MLW_1 |
> -                 PCI_EXP_LNK_LS_25);
> +                 QEMU_PCI_EXP_LNKCAP_MLW(QEMU_PCI_EXP_LNK_X1) |
> +                 QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT));
>  
>      pci_set_word(exp_cap + PCI_EXP_LNKSTA,
> -                 PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
> +                 QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1) |
> +                 QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT));
>  
>      if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
>          pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 5c7bd9698496..74f9a46b4be0 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1897,7 +1897,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>                                     PCI_EXP_TYPE_ENDPOINT << 4,
>                                     PCI_EXP_FLAGS_TYPE);
>              vfio_add_emulated_long(vdev, pos + PCI_EXP_LNKCAP,
> -                                   PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25, ~0);
> +                           QEMU_PCI_EXP_LNKCAP_MLW(QEMU_PCI_EXP_LNK_X1) |
> +                           QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT), ~0);
>              vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKCTL, 0, ~0);
>          }
>  
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index a95522a13b04..ad4e7808b8ac 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -34,10 +34,29 @@
>  
>  /* PCI_EXP_LINK{CAP, STA} */
>  /* link speed */
> -#define PCI_EXP_LNK_LS_25               1
> +typedef enum PCIExpLinkSpeed {
> +    QEMU_PCI_EXP_LNK_2_5GT = 1,
> +    QEMU_PCI_EXP_LNK_5GT,
> +    QEMU_PCI_EXP_LNK_8GT,
> +    QEMU_PCI_EXP_LNK_16GT,
> +} PCIExpLinkSpeed;
> +
> +#define QEMU_PCI_EXP_LNKCAP_MLS(speed)  (speed)
> +#define QEMU_PCI_EXP_LNKSTA_CLS         QEMU_PCI_EXP_LNKCAP_MLS
> +
> +typedef enum PCIExpLinkWidth {
> +    QEMU_PCI_EXP_LNK_X1 = 1,
> +    QEMU_PCI_EXP_LNK_X2 = 2,
> +    QEMU_PCI_EXP_LNK_X4 = 4,
> +    QEMU_PCI_EXP_LNK_X8 = 8,
> +    QEMU_PCI_EXP_LNK_X12 = 12,
> +    QEMU_PCI_EXP_LNK_X16 = 16,
> +    QEMU_PCI_EXP_LNK_X32 = 32,
> +} PCIExpLinkWidth;
>  
>  #define PCI_EXP_LNK_MLW_SHIFT           ctz32(PCI_EXP_LNKCAP_MLW)
> -#define PCI_EXP_LNK_MLW_1               (1 << PCI_EXP_LNK_MLW_SHIFT)
> +#define QEMU_PCI_EXP_LNKCAP_MLW(width)  (width << PCI_EXP_LNK_MLW_SHIFT)
> +#define QEMU_PCI_EXP_LNKSTA_NLW         QEMU_PCI_EXP_LNKCAP_MLW
>  
>  /* PCI_EXP_LINKCAP */
>  #define PCI_EXP_LNKCAP_ASPMS_SHIFT      ctz32(PCI_EXP_LNKCAP_ASPMS)
> 
> 

Re: [Qemu-devel] [for-4.0 PATCH v3 1/9] pcie: Create enums for link speed and width

[Auger Eric]

Hi

On 12/4/18 5:26 PM, Alex Williamson wrote:
> In preparation for reporting higher virtual link speeds and widths,
> create enums and macros to help us manage them.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  hw/pci/pcie.c              |    7 ++++---
>  hw/vfio/pci.c              |    3 ++-
>  include/hw/pci/pcie_regs.h |   23 +++++++++++++++++++++--
>  3 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 6c91bd44a0a5..914a5261a79b 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -68,11 +68,12 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
>      pci_set_long(exp_cap + PCI_EXP_LNKCAP,
>                   (port << PCI_EXP_LNKCAP_PN_SHIFT) |
>                   PCI_EXP_LNKCAP_ASPMS_0S |
> -                 PCI_EXP_LNK_MLW_1 |
> -                 PCI_EXP_LNK_LS_25);
> +                 QEMU_PCI_EXP_LNKCAP_MLW(QEMU_PCI_EXP_LNK_X1) |
> +                 QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT));
>  
>      pci_set_word(exp_cap + PCI_EXP_LNKSTA,
> -                 PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
> +                 QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1) |
> +                 QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT));
>  
>      if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
>          pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 5c7bd9698496..74f9a46b4be0 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1897,7 +1897,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>                                     PCI_EXP_TYPE_ENDPOINT << 4,
>                                     PCI_EXP_FLAGS_TYPE);
>              vfio_add_emulated_long(vdev, pos + PCI_EXP_LNKCAP,
> -                                   PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25, ~0);
> +                           QEMU_PCI_EXP_LNKCAP_MLW(QEMU_PCI_EXP_LNK_X1) |
> +                           QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT), ~0);
>              vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKCTL, 0, ~0);
>          }
>  
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index a95522a13b04..ad4e7808b8ac 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -34,10 +34,29 @@
>  
>  /* PCI_EXP_LINK{CAP, STA} */
>  /* link speed */
> -#define PCI_EXP_LNK_LS_25               1
> +typedef enum PCIExpLinkSpeed {
> +    QEMU_PCI_EXP_LNK_2_5GT = 1,
> +    QEMU_PCI_EXP_LNK_5GT,
> +    QEMU_PCI_EXP_LNK_8GT,
> +    QEMU_PCI_EXP_LNK_16GT,
> +} PCIExpLinkSpeed;
> +
> +#define QEMU_PCI_EXP_LNKCAP_MLS(speed)  (speed)
> +#define QEMU_PCI_EXP_LNKSTA_CLS         QEMU_PCI_EXP_LNKCAP_MLS
> +
> +typedef enum PCIExpLinkWidth {
> +    QEMU_PCI_EXP_LNK_X1 = 1,
> +    QEMU_PCI_EXP_LNK_X2 = 2,
> +    QEMU_PCI_EXP_LNK_X4 = 4,
> +    QEMU_PCI_EXP_LNK_X8 = 8,
> +    QEMU_PCI_EXP_LNK_X12 = 12,
> +    QEMU_PCI_EXP_LNK_X16 = 16,
> +    QEMU_PCI_EXP_LNK_X32 = 32,
> +} PCIExpLinkWidth;
>  
>  #define PCI_EXP_LNK_MLW_SHIFT           ctz32(PCI_EXP_LNKCAP_MLW)
> -#define PCI_EXP_LNK_MLW_1               (1 << PCI_EXP_LNK_MLW_SHIFT)
> +#define QEMU_PCI_EXP_LNKCAP_MLW(width)  (width << PCI_EXP_LNK_MLW_SHIFT)
> +#define QEMU_PCI_EXP_LNKSTA_NLW         QEMU_PCI_EXP_LNKCAP_MLW
>  
>  /* PCI_EXP_LINKCAP */
>  #define PCI_EXP_LNKCAP_ASPMS_SHIFT      ctz32(PCI_EXP_LNKCAP_ASPMS)
> 
> 

[Qemu-devel] [for-4.0 PATCH v3 2/9] pci: Sync PCIe downstream port LNKSTA on read

[Alex Williamson]

The PCIe link speed and width between a downstream device and its
upstream port is negotiated on real hardware and susceptible to
dynamic changes due to signal issues and power management.  In the
emulated device case there is no real hardware link, but we still
might wish to have some consistency between endpoint and downstream
port via a virtual negotiation.  There is of course a real link for
assigned devices and this same virtual negotiation allows the
downstream port to match the endpoint, synchronizing on every read
to support underlying physical hardware dynamically adjusting the
link.

This negotiation is intentionally unidirectional for compatibility.
If the endpoint exceeds the capabilities of the downstream port or
there is no endpoint device, the downstream port reports negotiation
to its maximum speed and width, matching the previous case where
negotiation was absent.  De-tuning the endpoint to match a virtual
link doesn't seem to benefit anyone and is a condition we've thus
far reported without functional issues.

Note that PCI_EXP_LNKSTA is already ignored for migration
compatibility via pcie_cap_v1_fill().

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci/pci.c          |    4 ++++
 hw/pci/pcie.c         |   39 +++++++++++++++++++++++++++++++++++++++
 include/hw/pci/pci.h  |   13 +++++++++++++
 include/hw/pci/pcie.h |    1 +
 4 files changed, 57 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 56b13b3320ec..495db3b9e18a 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1353,6 +1353,10 @@ uint32_t pci_default_read_config(PCIDevice *d,
 {
     uint32_t val = 0;
 
+    if (pci_is_express_downstream_port(d) &&
+        ranges_overlap(address, len, d->exp.exp_cap + PCI_EXP_LNKSTA, 2)) {
+        pcie_sync_bridge_lnk(d);
+    }
     memcpy(&val, d->config + address, len);
     return le32_to_cpu(val);
 }
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 914a5261a79b..61b7b96c52cd 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -729,6 +729,45 @@ void pcie_add_capability(PCIDevice *dev,
     memset(dev->cmask + offset, 0xFF, size);
 }
 
+/*
+ * Sync the PCIe Link Status negotiated speed and width of a bridge with the
+ * downstream device.  If downstream device is not present, re-write with the
+ * Link Capability fields.  Limit width and speed to bridge capabilities for
+ * compatibility.  Use config_read to access the downstream device since it
+ * could be an assigned device with volatile link information.
+ */
+void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
+{
+    PCIBridge *br = PCI_BRIDGE(bridge_dev);
+    PCIBus *bus = pci_bridge_get_sec_bus(br);
+    PCIDevice *target = bus->devices[0];
+    uint8_t *exp_cap = bridge_dev->config + bridge_dev->exp.exp_cap;
+    uint16_t lnksta, lnkcap = pci_get_word(exp_cap + PCI_EXP_LNKCAP);
+
+    if (!target || !target->exp.exp_cap) {
+        lnksta = lnkcap;
+    } else {
+        lnksta = target->config_read(target,
+                                     target->exp.exp_cap + PCI_EXP_LNKSTA,
+                                     sizeof(lnksta));
+
+        if ((lnksta & PCI_EXP_LNKSTA_NLW) > (lnkcap & PCI_EXP_LNKCAP_MLW)) {
+            lnksta &= ~PCI_EXP_LNKSTA_NLW;
+            lnksta |= lnkcap & PCI_EXP_LNKCAP_MLW;
+        }
+
+        if ((lnksta & PCI_EXP_LNKSTA_CLS) > (lnkcap & PCI_EXP_LNKCAP_SLS)) {
+            lnksta &= ~PCI_EXP_LNKSTA_CLS;
+            lnksta |= lnkcap & PCI_EXP_LNKCAP_SLS;
+        }
+    }
+
+    pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA,
+                                 PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW);
+    pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, lnksta &
+                               (PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW));
+}
+
 /**************************************************************************
  * pci express extended capability helper functions
  */
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e6514bba23aa..eb12fa112ed2 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -737,6 +737,19 @@ static inline int pci_is_express(const PCIDevice *d)
     return d->cap_present & QEMU_PCI_CAP_EXPRESS;
 }
 
+static inline int pci_is_express_downstream_port(const PCIDevice *d)
+{
+    uint8_t type;
+
+    if (!pci_is_express(d) || !d->exp.exp_cap) {
+        return 0;
+    }
+
+    type = pcie_cap_get_type(d);
+
+    return type == PCI_EXP_TYPE_DOWNSTREAM || type == PCI_EXP_TYPE_ROOT_PORT;
+}
+
 static inline uint32_t pci_config_size(const PCIDevice *d)
 {
     return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index b71e36970345..1976909ab4c8 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -126,6 +126,7 @@ uint16_t pcie_find_capability(PCIDevice *dev, uint16_t cap_id);
 void pcie_add_capability(PCIDevice *dev,
                          uint16_t cap_id, uint8_t cap_ver,
                          uint16_t offset, uint16_t size);
+void pcie_sync_bridge_lnk(PCIDevice *dev);
 
 void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
 void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);

Re: [Qemu-devel] [for-4.0 PATCH v3 2/9] pci: Sync PCIe downstream port LNKSTA on read

[Auger Eric]

Hi Alex,

On 12/4/18 5:26 PM, Alex Williamson wrote:
> The PCIe link speed and width between a downstream device and its
> upstream port is negotiated on real hardware and susceptible to
> dynamic changes due to signal issues and power management.  In the
> emulated device case there is no real hardware link, but we still
> might wish to have some consistency between endpoint and downstream
> port via a virtual negotiation.  There is of course a real link for
> assigned devices and this same virtual negotiation allows the
> downstream port to match the endpoint, synchronizing on every read
> to support underlying physical hardware dynamically adjusting the
> link.
> 
> This negotiation is intentionally unidirectional for compatibility.
> If the endpoint exceeds the capabilities of the downstream port or
> there is no endpoint device, the downstream port reports negotiation
> to its maximum speed and width, matching the previous case where
> negotiation was absent.  De-tuning the endpoint to match a virtual
> link doesn't seem to benefit anyone and is a condition we've thus
> far reported without functional issues.
> 
> Note that PCI_EXP_LNKSTA is already ignored for migration
> compatibility via pcie_cap_v1_fill().
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  hw/pci/pci.c          |    4 ++++
>  hw/pci/pcie.c         |   39 +++++++++++++++++++++++++++++++++++++++
>  include/hw/pci/pci.h  |   13 +++++++++++++
>  include/hw/pci/pcie.h |    1 +
>  4 files changed, 57 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 56b13b3320ec..495db3b9e18a 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1353,6 +1353,10 @@ uint32_t pci_default_read_config(PCIDevice *d,
>  {
>      uint32_t val = 0;
>  
> +    if (pci_is_express_downstream_port(d) &&
> +        ranges_overlap(address, len, d->exp.exp_cap + PCI_EXP_LNKSTA, 2)) {
> +        pcie_sync_bridge_lnk(d);
> +    }
>      memcpy(&val, d->config + address, len);
>      return le32_to_cpu(val);
>  }
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 914a5261a79b..61b7b96c52cd 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -729,6 +729,45 @@ void pcie_add_capability(PCIDevice *dev,
>      memset(dev->cmask + offset, 0xFF, size);
>  }
>  
> +/*
> + * Sync the PCIe Link Status negotiated speed and width of a bridge with the
> + * downstream device.  If downstream device is not present, re-write with the
> + * Link Capability fields.  Limit width and speed to bridge capabilities for
> + * compatibility.  Use config_read to access the downstream device since it
> + * could be an assigned device with volatile link information.
> + */
> +void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
> +{
> +    PCIBridge *br = PCI_BRIDGE(bridge_dev);
> +    PCIBus *bus = pci_bridge_get_sec_bus(br);
> +    PCIDevice *target = bus->devices[0];
> +    uint8_t *exp_cap = bridge_dev->config + bridge_dev->exp.exp_cap;
> +    uint16_t lnksta, lnkcap = pci_get_word(exp_cap + PCI_EXP_LNKCAP);
> +
> +    if (!target || !target->exp.exp_cap) {
> +        lnksta = lnkcap;
> +    } else {
> +        lnksta = target->config_read(target,
> +                                     target->exp.exp_cap + PCI_EXP_LNKSTA,
> +                                     sizeof(lnksta));
> +
> +        if ((lnksta & PCI_EXP_LNKSTA_NLW) > (lnkcap & PCI_EXP_LNKCAP_MLW)) {
> +            lnksta &= ~PCI_EXP_LNKSTA_NLW;
> +            lnksta |= lnkcap & PCI_EXP_LNKCAP_MLW;
> +        }
> +
> +        if ((lnksta & PCI_EXP_LNKSTA_CLS) > (lnkcap & PCI_EXP_LNKCAP_SLS)) {
> +            lnksta &= ~PCI_EXP_LNKSTA_CLS;
> +            lnksta |= lnkcap & PCI_EXP_LNKCAP_SLS;
> +        }
> +    }
> +
> +    pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA,
> +                                 PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW);
> +    pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, lnksta &
> +                               (PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW));
> +}
> +
>  /**************************************************************************
>   * pci express extended capability helper functions
>   */
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index e6514bba23aa..eb12fa112ed2 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -737,6 +737,19 @@ static inline int pci_is_express(const PCIDevice *d)
>      return d->cap_present & QEMU_PCI_CAP_EXPRESS;
>  }
>  
> +static inline int pci_is_express_downstream_port(const PCIDevice *d)
> +{
> +    uint8_t type;
> +
> +    if (!pci_is_express(d) || !d->exp.exp_cap) {
> +        return 0;
> +    }
> +
> +    type = pcie_cap_get_type(d);
> +
> +    return type == PCI_EXP_TYPE_DOWNSTREAM || type == PCI_EXP_TYPE_ROOT_PORT;
> +}
> +
>  static inline uint32_t pci_config_size(const PCIDevice *d)
>  {
>      return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index b71e36970345..1976909ab4c8 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -126,6 +126,7 @@ uint16_t pcie_find_capability(PCIDevice *dev, uint16_t cap_id);
>  void pcie_add_capability(PCIDevice *dev,
>                           uint16_t cap_id, uint8_t cap_ver,
>                           uint16_t offset, uint16_t size);
> +void pcie_sync_bridge_lnk(PCIDevice *dev);
>  
>  void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
>  void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> 
> 

[Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Alex Williamson]

Create properties to be able to define speeds and widths for PCIe
links.  The only tricky bit here is that our get and set callbacks
translate from the fixed QAPI automagic enums to those we define
in PCI code to represent the actual register segment value.

Cc: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/core/qdev-properties.c    |  178 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/qdev-properties.h |    8 ++
 qapi/common.json             |   42 ++++++++++
 3 files changed, 228 insertions(+)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 35072dec1ecf..f5ca5b821a79 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
     .set = set_enum,
     .set_default_value = set_default_value_enum,
 };
+
+/* --- PCIELinkSpeed 2_5/5/8/16 -- */
+
+static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
+    PCIELinkSpeed speed;
+
+    switch (*p) {
+    case QEMU_PCI_EXP_LNK_2_5GT:
+        speed = PCIE_LINK_SPEED_2_5;
+        break;
+    case QEMU_PCI_EXP_LNK_5GT:
+        speed = PCIE_LINK_SPEED_5;
+        break;
+    case QEMU_PCI_EXP_LNK_8GT:
+        speed = PCIE_LINK_SPEED_8;
+        break;
+    case QEMU_PCI_EXP_LNK_16GT:
+        speed = PCIE_LINK_SPEED_16;
+        break;
+    default:
+        /* Unreachable */
+        abort();
+    }
+
+    visit_type_enum(v, prop->name, (int *)&speed, prop->info->enum_table, errp);
+}
+
+static void set_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
+    PCIELinkSpeed speed;
+    Error *local_err = NULL;
+
+    if (dev->realized) {
+        qdev_prop_set_after_realize(dev, name, errp);
+        return;
+    }
+
+    visit_type_enum(v, prop->name, (int *)&speed,
+                    prop->info->enum_table, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    switch (speed) {
+    case PCIE_LINK_SPEED_2_5:
+        *p = QEMU_PCI_EXP_LNK_2_5GT;
+        break;
+    case PCIE_LINK_SPEED_5:
+        *p = QEMU_PCI_EXP_LNK_5GT;
+        break;
+    case PCIE_LINK_SPEED_8:
+        *p = QEMU_PCI_EXP_LNK_8GT;
+        break;
+    case PCIE_LINK_SPEED_16:
+        *p = QEMU_PCI_EXP_LNK_16GT;
+        break;
+    default:
+        /* Unreachable */
+        abort();
+    }
+}
+
+const PropertyInfo qdev_prop_pcie_link_speed = {
+    .name = "PCIELinkSpeed",
+    .description = "2_5/5/8/16",
+    .enum_table = &PCIELinkSpeed_lookup,
+    .get = get_prop_pcielinkspeed,
+    .set = set_prop_pcielinkspeed,
+    .set_default_value = set_default_value_enum,
+};
+
+/* --- PCIELinkWidth 1/2/4/8/12/16/32 -- */
+
+static void get_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    PCIExpLinkWidth *p = qdev_get_prop_ptr(dev, prop);
+    PCIELinkWidth width;
+
+    switch (*p) {
+    case QEMU_PCI_EXP_LNK_X1:
+        width = PCIE_LINK_WIDTH_1;
+        break;
+    case QEMU_PCI_EXP_LNK_X2:
+        width = PCIE_LINK_WIDTH_2;
+        break;
+    case QEMU_PCI_EXP_LNK_X4:
+        width = PCIE_LINK_WIDTH_4;
+        break;
+    case QEMU_PCI_EXP_LNK_X8:
+        width = PCIE_LINK_WIDTH_8;
+        break;
+    case QEMU_PCI_EXP_LNK_X12:
+        width = PCIE_LINK_WIDTH_12;
+        break;
+    case QEMU_PCI_EXP_LNK_X16:
+        width = PCIE_LINK_WIDTH_16;
+        break;
+    case QEMU_PCI_EXP_LNK_X32:
+        width = PCIE_LINK_WIDTH_32;
+        break;
+    default:
+        /* Unreachable */
+        abort();
+    }
+
+    visit_type_enum(v, prop->name, (int *)&width, prop->info->enum_table, errp);
+}
+
+static void set_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    PCIExpLinkWidth *p = qdev_get_prop_ptr(dev, prop);
+    PCIELinkWidth width;
+    Error *local_err = NULL;
+
+    if (dev->realized) {
+        qdev_prop_set_after_realize(dev, name, errp);
+        return;
+    }
+
+    visit_type_enum(v, prop->name, (int *)&width,
+                    prop->info->enum_table, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    switch (width) {
+    case PCIE_LINK_WIDTH_1:
+        *p = QEMU_PCI_EXP_LNK_X1;
+        break;
+    case PCIE_LINK_WIDTH_2:
+        *p = QEMU_PCI_EXP_LNK_X2;
+        break;
+    case PCIE_LINK_WIDTH_4:
+        *p = QEMU_PCI_EXP_LNK_X4;
+        break;
+    case PCIE_LINK_WIDTH_8:
+        *p = QEMU_PCI_EXP_LNK_X8;
+        break;
+    case PCIE_LINK_WIDTH_12:
+        *p = QEMU_PCI_EXP_LNK_X12;
+        break;
+    case PCIE_LINK_WIDTH_16:
+        *p = QEMU_PCI_EXP_LNK_X16;
+        break;
+    case PCIE_LINK_WIDTH_32:
+        *p = QEMU_PCI_EXP_LNK_X32;
+        break;
+    default:
+        /* Unreachable */
+        abort();
+    }
+}
+
+const PropertyInfo qdev_prop_pcie_link_width = {
+    .name = "PCIELinkWidth",
+    .description = "1/2/4/8/12/16/32",
+    .enum_table = &PCIELinkWidth_lookup,
+    .get = get_prop_pcielinkwidth,
+    .set = set_prop_pcielinkwidth,
+    .set_default_value = set_default_value_enum,
+};
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 4f60cc88f325..6a13a284c48c 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -36,6 +36,8 @@ extern const PropertyInfo qdev_prop_uuid;
 extern const PropertyInfo qdev_prop_arraylen;
 extern const PropertyInfo qdev_prop_link;
 extern const PropertyInfo qdev_prop_off_auto_pcibar;
+extern const PropertyInfo qdev_prop_pcie_link_speed;
+extern const PropertyInfo qdev_prop_pcie_link_width;
 
 #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
         .name      = (_name),                                    \
@@ -217,6 +219,12 @@ extern const PropertyInfo qdev_prop_off_auto_pcibar;
 #define DEFINE_PROP_OFF_AUTO_PCIBAR(_n, _s, _f, _d) \
     DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_off_auto_pcibar, \
                         OffAutoPCIBAR)
+#define DEFINE_PROP_PCIE_LINK_SPEED(_n, _s, _f, _d) \
+    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pcie_link_speed, \
+                        PCIExpLinkSpeed)
+#define DEFINE_PROP_PCIE_LINK_WIDTH(_n, _s, _f, _d) \
+    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pcie_link_width, \
+                        PCIExpLinkWidth)
 
 #define DEFINE_PROP_UUID(_name, _state, _field) {                  \
         .name      = (_name),                                      \
diff --git a/qapi/common.json b/qapi/common.json
index 021174f04ea4..b6f3cca35c7e 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -127,6 +127,48 @@
 { 'enum': 'OffAutoPCIBAR',
   'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
 
+##
+# @PCIELinkSpeed:
+#
+# An enumeration of PCIe link speeds in units of GT/s
+#
+# @2_5: 2.5GT/s
+#
+# @5: 5.0GT/s
+#
+# @8: 8.0GT/s
+#
+# @16: 16.0GT/s
+#
+# Since: 3.2
+##
+{ 'enum': 'PCIELinkSpeed',
+  'data': [ '2_5', '5', '8', '16' ] }
+
+##
+# @PCIELinkWidth:
+#
+# An enumeration of PCIe link width
+#
+# @1: x1
+#
+# @2: x2
+#
+# @4: x4
+#
+# @8: x8
+#
+# @12: x12
+#
+# @16: x16
+#
+# @32: x32
+#
+# Since: 3.2
+##
+{ 'enum': 'PCIELinkWidth',
+  'data': [ '1', '2', '4', '8', '12', '16', '32' ] }
+
 ##
 # @SysEmuTarget:
 #

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Markus Armbruster]

Alex Williamson <alex.williamson@redhat.com> writes:

> Create properties to be able to define speeds and widths for PCIe
> links.  The only tricky bit here is that our get and set callbacks
> translate from the fixed QAPI automagic enums to those we define
> in PCI code to represent the actual register segment value.

QAPI can only generate enumerations with values 0, 1, 2, ...  You want
different enumeration values, namely the actual register values.  You
still want QAPI to get its standard mapping to and from strings.

This patch's solution is to define a non-QAPI enumeration type with the
values you want [PATCH 1/9], then map between the enumerations in the
PropertyInfo methods.  Works.

You could instead use the encoding chosen by QAPI for the properties,
and map it to the register values on use.  Differently ugly.  Might be
simpler.  Your choice to make.

We could extend QAPI to permit specification of the enumeration values.
Marc-André's work to permit conditionals makes the syntax flexible
enough to support that.  Of course, adding QAPI features is worthwhile
only if we get sufficient mileage out of them to result in an overall
improvement.  Even if we decided to do it right now, I'd recommend not
to wait for it, but instead plan to simplify after the feature lands.

> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Alex Williamson]

On Wed, 05 Dec 2018 10:09:38 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Alex Williamson <alex.williamson@redhat.com> writes:
> 
> > Create properties to be able to define speeds and widths for PCIe
> > links.  The only tricky bit here is that our get and set callbacks
> > translate from the fixed QAPI automagic enums to those we define
> > in PCI code to represent the actual register segment value.  
> 
> QAPI can only generate enumerations with values 0, 1, 2, ...  You want
> different enumeration values, namely the actual register values.  You
> still want QAPI to get its standard mapping to and from strings.
> 
> This patch's solution is to define a non-QAPI enumeration type with the
> values you want [PATCH 1/9], then map between the enumerations in the
> PropertyInfo methods.  Works.
> 
> You could instead use the encoding chosen by QAPI for the properties,
> and map it to the register values on use.  Differently ugly.  Might be
> simpler.  Your choice to make.

As we discussed offline, I personally don't like using the magic macros
that QAPI generates outside of QAPI code; I can't git grep for them and
I need to remember to build the tree before using cscope in order to
find them.  Therefore I prefer to keep the translation to standard
macros within the QAPI code to make things more obvious, thus the
approach chosen.  I appreciate the feedback and alternative ideas.

> We could extend QAPI to permit specification of the enumeration values.
> Marc-André's work to permit conditionals makes the syntax flexible
> enough to support that.  Of course, adding QAPI features is worthwhile
> only if we get sufficient mileage out of them to result in an overall
> improvement.  Even if we decided to do it right now, I'd recommend not
> to wait for it, but instead plan to simplify after the feature lands.

Good to know.

> > Cc: Eric Blake <eblake@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Tested-by: Geoffrey McRae <geoff@hostfission.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>  
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks!

Alex

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Auger Eric]

Hi Alex,

On 12/4/18 5:26 PM, Alex Williamson wrote:
> Create properties to be able to define speeds and widths for PCIe
> links.  The only tricky bit here is that our get and set callbacks
> translate from the fixed QAPI automagic enums to those we define
> in PCI code to represent the actual register segment value.
> 
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/core/qdev-properties.c    |  178 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/qdev-properties.h |    8 ++
>  qapi/common.json             |   42 ++++++++++
>  3 files changed, 228 insertions(+)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 35072dec1ecf..f5ca5b821a79 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
>      .set = set_enum,
>      .set_default_value = set_default_value_enum,
>  };
> +
> +/* --- PCIELinkSpeed 2_5/5/8/16 -- */
> +
> +static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
> +                                   void *opaque, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
> +    PCIELinkSpeed speed;
> +
> +    switch (*p) {
> +    case QEMU_PCI_EXP_LNK_2_5GT:
> +        speed = PCIE_LINK_SPEED_2_5;
> +        break;
> +    case QEMU_PCI_EXP_LNK_5GT:
> +        speed = PCIE_LINK_SPEED_5;
> +        break;
> +    case QEMU_PCI_EXP_LNK_8GT:
> +        speed = PCIE_LINK_SPEED_8;
> +        break;
> +    case QEMU_PCI_EXP_LNK_16GT:
> +        speed = PCIE_LINK_SPEED_16;
> +        break;
> +    default:
> +        /* Unreachable */
> +        abort();
nit: g_assert_not_reached() here and below.
> +    }
> +
> +    visit_type_enum(v, prop->name, (int *)&speed, prop->info->enum_table, errp);
> +}
> +
> +static void set_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
> +                                   void *opaque, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
> +    PCIELinkSpeed speed;
> +    Error *local_err = NULL;
> +
> +    if (dev->realized) {
> +        qdev_prop_set_after_realize(dev, name, errp);
> +        return;
> +    }
> +
> +    visit_type_enum(v, prop->name, (int *)&speed,
> +                    prop->info->enum_table, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    switch (speed) {
> +    case PCIE_LINK_SPEED_2_5:
> +        *p = QEMU_PCI_EXP_LNK_2_5GT;
> +        break;
> +    case PCIE_LINK_SPEED_5:
> +        *p = QEMU_PCI_EXP_LNK_5GT;
> +        break;
> +    case PCIE_LINK_SPEED_8:
> +        *p = QEMU_PCI_EXP_LNK_8GT;
> +        break;
> +    case PCIE_LINK_SPEED_16:
> +        *p = QEMU_PCI_EXP_LNK_16GT;
> +        break;
> +    default:
> +        /* Unreachable */
> +        abort();
> +    }
> +}
> +
> +const PropertyInfo qdev_prop_pcie_link_speed = {
> +    .name = "PCIELinkSpeed",
> +    .description = "2_5/5/8/16",
> +    .enum_table = &PCIELinkSpeed_lookup,
> +    .get = get_prop_pcielinkspeed,
> +    .set = set_prop_pcielinkspeed,
> +    .set_default_value = set_default_value_enum,
> +};
> +
> +/* --- PCIELinkWidth 1/2/4/8/12/16/32 -- */
> +
> +static void get_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
> +                                   void *opaque, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    PCIExpLinkWidth *p = qdev_get_prop_ptr(dev, prop);
> +    PCIELinkWidth width;
> +
> +    switch (*p) {
> +    case QEMU_PCI_EXP_LNK_X1:
> +        width = PCIE_LINK_WIDTH_1;
> +        break;
> +    case QEMU_PCI_EXP_LNK_X2:
> +        width = PCIE_LINK_WIDTH_2;
> +        break;
> +    case QEMU_PCI_EXP_LNK_X4:
> +        width = PCIE_LINK_WIDTH_4;
> +        break;
> +    case QEMU_PCI_EXP_LNK_X8:
> +        width = PCIE_LINK_WIDTH_8;
> +        break;
> +    case QEMU_PCI_EXP_LNK_X12:
> +        width = PCIE_LINK_WIDTH_12;
> +        break;
> +    case QEMU_PCI_EXP_LNK_X16:
> +        width = PCIE_LINK_WIDTH_16;
> +        break;
> +    case QEMU_PCI_EXP_LNK_X32:
> +        width = PCIE_LINK_WIDTH_32;
> +        break;
> +    default:
> +        /* Unreachable */
> +        abort();
> +    }
> +
> +    visit_type_enum(v, prop->name, (int *)&width, prop->info->enum_table, errp);
> +}
> +
> +static void set_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
> +                                   void *opaque, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    PCIExpLinkWidth *p = qdev_get_prop_ptr(dev, prop);
> +    PCIELinkWidth width;
> +    Error *local_err = NULL;
> +
> +    if (dev->realized) {
> +        qdev_prop_set_after_realize(dev, name, errp);
> +        return;
> +    }
> +
> +    visit_type_enum(v, prop->name, (int *)&width,
> +                    prop->info->enum_table, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    switch (width) {
> +    case PCIE_LINK_WIDTH_1:
> +        *p = QEMU_PCI_EXP_LNK_X1;
> +        break;
> +    case PCIE_LINK_WIDTH_2:
> +        *p = QEMU_PCI_EXP_LNK_X2;
> +        break;
> +    case PCIE_LINK_WIDTH_4:
> +        *p = QEMU_PCI_EXP_LNK_X4;
> +        break;
> +    case PCIE_LINK_WIDTH_8:
> +        *p = QEMU_PCI_EXP_LNK_X8;
> +        break;
> +    case PCIE_LINK_WIDTH_12:
> +        *p = QEMU_PCI_EXP_LNK_X12;
> +        break;
> +    case PCIE_LINK_WIDTH_16:
> +        *p = QEMU_PCI_EXP_LNK_X16;
> +        break;
> +    case PCIE_LINK_WIDTH_32:
> +        *p = QEMU_PCI_EXP_LNK_X32;
> +        break;
> +    default:
> +        /* Unreachable */
> +        abort();
> +    }
> +}
> +
> +const PropertyInfo qdev_prop_pcie_link_width = {
> +    .name = "PCIELinkWidth",
> +    .description = "1/2/4/8/12/16/32",
> +    .enum_table = &PCIELinkWidth_lookup,
> +    .get = get_prop_pcielinkwidth,
> +    .set = set_prop_pcielinkwidth,
> +    .set_default_value = set_default_value_enum,
> +};
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 4f60cc88f325..6a13a284c48c 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -36,6 +36,8 @@ extern const PropertyInfo qdev_prop_uuid;
>  extern const PropertyInfo qdev_prop_arraylen;
>  extern const PropertyInfo qdev_prop_link;
>  extern const PropertyInfo qdev_prop_off_auto_pcibar;
> +extern const PropertyInfo qdev_prop_pcie_link_speed;
> +extern const PropertyInfo qdev_prop_pcie_link_width;
>  
>  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
>          .name      = (_name),                                    \
> @@ -217,6 +219,12 @@ extern const PropertyInfo qdev_prop_off_auto_pcibar;
>  #define DEFINE_PROP_OFF_AUTO_PCIBAR(_n, _s, _f, _d) \
>      DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_off_auto_pcibar, \
>                          OffAutoPCIBAR)
> +#define DEFINE_PROP_PCIE_LINK_SPEED(_n, _s, _f, _d) \
> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pcie_link_speed, \
> +                        PCIExpLinkSpeed)
> +#define DEFINE_PROP_PCIE_LINK_WIDTH(_n, _s, _f, _d) \
> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pcie_link_width, \
> +                        PCIExpLinkWidth)
>  
>  #define DEFINE_PROP_UUID(_name, _state, _field) {                  \
>          .name      = (_name),                                      \
> diff --git a/qapi/common.json b/qapi/common.json
> index 021174f04ea4..b6f3cca35c7e 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -127,6 +127,48 @@
>  { 'enum': 'OffAutoPCIBAR',
>    'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
>  
> +##
> +# @PCIELinkSpeed:
> +#
> +# An enumeration of PCIe link speeds in units of GT/s
> +#
> +# @2_5: 2.5GT/s
> +#
> +# @5: 5.0GT/s
> +#
> +# @8: 8.0GT/s
> +#
> +# @16: 16.0GT/s
> +#
> +# Since: 3.2
4.0 here and below

Thanks

Eric
> +##
> +{ 'enum': 'PCIELinkSpeed',
> +  'data': [ '2_5', '5', '8', '16' ] }
> +
> +##
> +# @PCIELinkWidth:
> +#
> +# An enumeration of PCIe link width
> +#
> +# @1: x1
> +#
> +# @2: x2
> +#
> +# @4: x4
> +#
> +# @8: x8
> +#
> +# @12: x12
> +#
> +# @16: x16
> +#
> +# @32: x32
> +#
> +# Since: 3.2
> +##
> +{ 'enum': 'PCIELinkWidth',
> +  'data': [ '1', '2', '4', '8', '12', '16', '32' ] }
> +
>  ##
>  # @SysEmuTarget:
>  #
> 
> 

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Markus Armbruster]

Auger Eric <eric.auger@redhat.com> writes:

> Hi Alex,
>
> On 12/4/18 5:26 PM, Alex Williamson wrote:
>> Create properties to be able to define speeds and widths for PCIe
>> links.  The only tricky bit here is that our get and set callbacks
>> translate from the fixed QAPI automagic enums to those we define
>> in PCI code to represent the actual register segment value.
>> 
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Tested-by: Geoffrey McRae <geoff@hostfission.com>
>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> ---
>>  hw/core/qdev-properties.c    |  178 ++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/qdev-properties.h |    8 ++
>>  qapi/common.json             |   42 ++++++++++
>>  3 files changed, 228 insertions(+)
>> 
>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 35072dec1ecf..f5ca5b821a79 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
>>      .set = set_enum,
>>      .set_default_value = set_default_value_enum,
>>  };
>> +
>> +/* --- PCIELinkSpeed 2_5/5/8/16 -- */
>> +
>> +static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
>> +                                   void *opaque, Error **errp)
>> +{
>> +    DeviceState *dev = DEVICE(obj);
>> +    Property *prop = opaque;
>> +    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
>> +    PCIELinkSpeed speed;
>> +
>> +    switch (*p) {
>> +    case QEMU_PCI_EXP_LNK_2_5GT:
>> +        speed = PCIE_LINK_SPEED_2_5;
>> +        break;
>> +    case QEMU_PCI_EXP_LNK_5GT:
>> +        speed = PCIE_LINK_SPEED_5;
>> +        break;
>> +    case QEMU_PCI_EXP_LNK_8GT:
>> +        speed = PCIE_LINK_SPEED_8;
>> +        break;
>> +    case QEMU_PCI_EXP_LNK_16GT:
>> +        speed = PCIE_LINK_SPEED_16;
>> +        break;
>> +    default:
>> +        /* Unreachable */
>> +        abort();
> nit: g_assert_not_reached() here and below.

In my opinion, g_assert_not_reached() & friends are an overly ornate
reinvention of an old and perfectly adequate wheel.

A long time ago for reasons since forgotten, the maintainers in charge
back then demanded abort() instead of assert(0).  Either is fine with
me.

I tolerate g_assert_not_reached() in files that already use g_assert().
This one doesn't.

In any case, I'd drop the comment.

Note that I'm not this file's maintainer.

[...]

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Auger Eric]

Hi Markus,

On 12/5/18 3:16 PM, Markus Armbruster wrote:
> Auger Eric <eric.auger@redhat.com> writes:
> 
>> Hi Alex,
>>
>> On 12/4/18 5:26 PM, Alex Williamson wrote:
>>> Create properties to be able to define speeds and widths for PCIe
>>> links.  The only tricky bit here is that our get and set callbacks
>>> translate from the fixed QAPI automagic enums to those we define
>>> in PCI code to represent the actual register segment value.
>>>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Tested-by: Geoffrey McRae <geoff@hostfission.com>
>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>>> ---
>>>  hw/core/qdev-properties.c    |  178 ++++++++++++++++++++++++++++++++++++++++++
>>>  include/hw/qdev-properties.h |    8 ++
>>>  qapi/common.json             |   42 ++++++++++
>>>  3 files changed, 228 insertions(+)
>>>
>>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>>> index 35072dec1ecf..f5ca5b821a79 100644
>>> --- a/hw/core/qdev-properties.c
>>> +++ b/hw/core/qdev-properties.c
>>> @@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
>>>      .set = set_enum,
>>>      .set_default_value = set_default_value_enum,
>>>  };
>>> +
>>> +/* --- PCIELinkSpeed 2_5/5/8/16 -- */
>>> +
>>> +static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
>>> +                                   void *opaque, Error **errp)
>>> +{
>>> +    DeviceState *dev = DEVICE(obj);
>>> +    Property *prop = opaque;
>>> +    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
>>> +    PCIELinkSpeed speed;
>>> +
>>> +    switch (*p) {
>>> +    case QEMU_PCI_EXP_LNK_2_5GT:
>>> +        speed = PCIE_LINK_SPEED_2_5;
>>> +        break;
>>> +    case QEMU_PCI_EXP_LNK_5GT:
>>> +        speed = PCIE_LINK_SPEED_5;
>>> +        break;
>>> +    case QEMU_PCI_EXP_LNK_8GT:
>>> +        speed = PCIE_LINK_SPEED_8;
>>> +        break;
>>> +    case QEMU_PCI_EXP_LNK_16GT:
>>> +        speed = PCIE_LINK_SPEED_16;
>>> +        break;
>>> +    default:
>>> +        /* Unreachable */
>>> +        abort();
>> nit: g_assert_not_reached() here and below.
> 
> In my opinion, g_assert_not_reached() & friends are an overly ornate
> reinvention of an old and perfectly adequate wheel.
> 
> A long time ago for reasons since forgotten, the maintainers in charge
> back then demanded abort() instead of assert(0).  Either is fine with
> me.
> 
> I tolerate g_assert_not_reached() in files that already use g_assert().
> This one doesn't.
> 
> In any case, I'd drop the comment.

OK I did not know. In the past I was encouraged to use it.

Thanks

Eric
> 
> Note that I'm not this file's maintainer.
> 
> [...]
> 

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Markus Armbruster]

Auger Eric <eric.auger@redhat.com> writes:

> Hi Markus,
>
> On 12/5/18 3:16 PM, Markus Armbruster wrote:
>> Auger Eric <eric.auger@redhat.com> writes:
[...]
>>> nit: g_assert_not_reached() here and below.
>> 
>> In my opinion, g_assert_not_reached() & friends are an overly ornate
>> reinvention of an old and perfectly adequate wheel.
>> 
>> A long time ago for reasons since forgotten, the maintainers in charge
>> back then demanded abort() instead of assert(0).  Either is fine with
>> me.
>> 
>> I tolerate g_assert_not_reached() in files that already use g_assert().
>> This one doesn't.
>> 
>> In any case, I'd drop the comment.
>
> OK I did not know. In the past I was encouraged to use it.

Some maintainers like ornate ;)

> Thanks
>
> Eric
>> 
>> Note that I'm not this file's maintainer.
>> 
>> [...]

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Alex Williamson]

On Wed, 05 Dec 2018 15:16:27 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Auger Eric <eric.auger@redhat.com> writes:
> 
> > Hi Alex,
> >
> > On 12/4/18 5:26 PM, Alex Williamson wrote:  
> >> Create properties to be able to define speeds and widths for PCIe
> >> links.  The only tricky bit here is that our get and set callbacks
> >> translate from the fixed QAPI automagic enums to those we define
> >> in PCI code to represent the actual register segment value.
> >> 
> >> Cc: Eric Blake <eblake@redhat.com>
> >> Cc: Markus Armbruster <armbru@redhat.com>
> >> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> >> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> >> ---
> >>  hw/core/qdev-properties.c    |  178 ++++++++++++++++++++++++++++++++++++++++++
> >>  include/hw/qdev-properties.h |    8 ++
> >>  qapi/common.json             |   42 ++++++++++
> >>  3 files changed, 228 insertions(+)
> >> 
> >> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> >> index 35072dec1ecf..f5ca5b821a79 100644
> >> --- a/hw/core/qdev-properties.c
> >> +++ b/hw/core/qdev-properties.c
> >> @@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
> >>      .set = set_enum,
> >>      .set_default_value = set_default_value_enum,
> >>  };
> >> +
> >> +/* --- PCIELinkSpeed 2_5/5/8/16 -- */
> >> +
> >> +static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
> >> +                                   void *opaque, Error **errp)
> >> +{
> >> +    DeviceState *dev = DEVICE(obj);
> >> +    Property *prop = opaque;
> >> +    PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
> >> +    PCIELinkSpeed speed;
> >> +
> >> +    switch (*p) {
> >> +    case QEMU_PCI_EXP_LNK_2_5GT:
> >> +        speed = PCIE_LINK_SPEED_2_5;
> >> +        break;
> >> +    case QEMU_PCI_EXP_LNK_5GT:
> >> +        speed = PCIE_LINK_SPEED_5;
> >> +        break;
> >> +    case QEMU_PCI_EXP_LNK_8GT:
> >> +        speed = PCIE_LINK_SPEED_8;
> >> +        break;
> >> +    case QEMU_PCI_EXP_LNK_16GT:
> >> +        speed = PCIE_LINK_SPEED_16;
> >> +        break;
> >> +    default:
> >> +        /* Unreachable */
> >> +        abort();  
> > nit: g_assert_not_reached() here and below.  
> 
> In my opinion, g_assert_not_reached() & friends are an overly ornate
> reinvention of an old and perfectly adequate wheel.
> 
> A long time ago for reasons since forgotten, the maintainers in charge
> back then demanded abort() instead of assert(0).  Either is fine with
> me.
> 
> I tolerate g_assert_not_reached() in files that already use g_assert().
> This one doesn't.
> 
> In any case, I'd drop the comment.

I added the comment because as a casual QAPI contributor it's otherwise
not obvious that bogus user input can't reach that case.  Comments are
free.
 
> Note that I'm not this file's maintainer.

get_maintainer.pl says there is no maintainer here and references:

"Michael S. Tsirkin" <mst@redhat.com> (commit_signer:3/4=75%)
"Marc-André Lureau" <marcandre.lureau@redhat.com> (commit_signer:2/4=50%)
Markus Armbruster <armbru@redhat.com> (commit_signer:1/4=25%)
"Philippe Mathieu-Daudé" <f4bug@amsat.org> (commit_signer:1/4=25%)
Paolo Bonzini <pbonzini@redhat.com> (commit_signer:1/4=25%)

CC'ing those folks.  Unless anyone expresses a strong opinion or trend
towards using g_assert_not_reached(), I'll stick with Markus' style to
only use it in files where g_assert() is already present... or at least
claim that's why I didn't use it ;)  Thanks,

Alex

Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties

[Alex Williamson]

On Wed, 5 Dec 2018 13:42:25 +0100
Auger Eric <eric.auger@redhat.com> wrote:

> > --- a/qapi/common.json
> > +++ b/qapi/common.json
> > @@ -127,6 +127,48 @@
> >  { 'enum': 'OffAutoPCIBAR',
> >    'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
> >  
> > +##
> > +# @PCIELinkSpeed:
> > +#
> > +# An enumeration of PCIe link speeds in units of GT/s
> > +#
> > +# @2_5: 2.5GT/s
> > +#
> > +# @5: 5.0GT/s
> > +#
> > +# @8: 8.0GT/s
> > +#
> > +# @16: 16.0GT/s
> > +#
> > +# Since: 3.2  
> 4.0 here and below

Fixed in the next spin.  Thanks,

Alex

> > +##
> > +{ 'enum': 'PCIELinkSpeed',
> > +  'data': [ '2_5', '5', '8', '16' ] }
> > +
> > +##
> > +# @PCIELinkWidth:
> > +#
> > +# An enumeration of PCIe link width
> > +#
> > +# @1: x1
> > +#
> > +# @2: x2
> > +#
> > +# @4: x4
> > +#
> > +# @8: x8
> > +#
> > +# @12: x12
> > +#
> > +# @16: x16
> > +#
> > +# @32: x32
> > +#
> > +# Since: 3.2
> > +##
> > +{ 'enum': 'PCIELinkWidth',
> > +  'data': [ '1', '2', '4', '8', '12', '16', '32' ] }
> > +
> >  ##
> >  # @SysEmuTarget:
> >  #
> > 
> >   

[Qemu-devel] [for-4.0 PATCH v3 4/9] pcie: Add link speed and width fields to PCIESlot

[Alex Williamson]

Add fields allowing the PCIe link speed and width of a PCIESlot to
be configured, with an instance_post_init callback on the root port
parent class to set defaults.  This allows child classes to set these
via properties or via their own instance_init callback, without
requiring all implementions to support arbitrary user selected values.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci-bridge/pcie_root_port.c |   14 ++++++++++++++
 include/hw/pci/pcie_port.h     |    4 ++++
 2 files changed, 18 insertions(+)

diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 45f9e8cd4a36..34ad76743c44 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -140,6 +140,19 @@ static Property rp_props[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
+static void rp_instance_post_init(Object *obj)
+{
+    PCIESlot *s = PCIE_SLOT(obj);
+
+    if (!s->speed) {
+        s->speed = QEMU_PCI_EXP_LNK_2_5GT;
+    }
+
+    if (!s->width) {
+        s->width = QEMU_PCI_EXP_LNK_X1;
+    }
+}
+
 static void rp_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -157,6 +170,7 @@ static void rp_class_init(ObjectClass *klass, void *data)
 static const TypeInfo rp_info = {
     .name          = TYPE_PCIE_ROOT_PORT,
     .parent        = TYPE_PCIE_SLOT,
+    .instance_post_init = rp_instance_post_init,
     .class_init    = rp_class_init,
     .abstract      = true,
     .class_size = sizeof(PCIERootPortClass),
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 0736014bfdb4..df242a0cafff 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -49,6 +49,10 @@ struct PCIESlot {
     /* pci express switch port with slot */
     uint8_t     chassis;
     uint16_t    slot;
+
+    PCIExpLinkSpeed speed;
+    PCIExpLinkWidth width;
+
     QLIST_ENTRY(PCIESlot) next;
 };
 

Re: [Qemu-devel] [for-4.0 PATCH v3 4/9] pcie: Add link speed and width fields to PCIESlot

[Auger Eric]

Hi,

On 12/4/18 5:26 PM, Alex Williamson wrote:
> Add fields allowing the PCIe link speed and width of a PCIESlot to
> be configured, with an instance_post_init callback on the root port
> parent class to set defaults.  This allows child classes to set these
> via properties or via their own instance_init callback, without
> requiring all implementions to support arbitrary user selected values.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/pci-bridge/pcie_root_port.c |   14 ++++++++++++++
>  include/hw/pci/pcie_port.h     |    4 ++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 45f9e8cd4a36..34ad76743c44 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -140,6 +140,19 @@ static Property rp_props[] = {
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> +static void rp_instance_post_init(Object *obj)
> +{
> +    PCIESlot *s = PCIE_SLOT(obj);
> +
> +    if (!s->speed) {
> +        s->speed = QEMU_PCI_EXP_LNK_2_5GT;
> +    }
> +
> +    if (!s->width) {
> +        s->width = QEMU_PCI_EXP_LNK_X1;
> +    }
> +}
> +
>  static void rp_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -157,6 +170,7 @@ static void rp_class_init(ObjectClass *klass, void *data)
>  static const TypeInfo rp_info = {
>      .name          = TYPE_PCIE_ROOT_PORT,
>      .parent        = TYPE_PCIE_SLOT,
> +    .instance_post_init = rp_instance_post_init,
>      .class_init    = rp_class_init,
>      .abstract      = true,
>      .class_size = sizeof(PCIERootPortClass),
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index 0736014bfdb4..df242a0cafff 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -49,6 +49,10 @@ struct PCIESlot {
>      /* pci express switch port with slot */
>      uint8_t     chassis;
>      uint16_t    slot;
> +
> +    PCIExpLinkSpeed speed;
> +    PCIExpLinkWidth width;
> +
>      QLIST_ENTRY(PCIESlot) next;
>  };
>  
> 
> 

[Qemu-devel] [for-4.0 PATCH v3 5/9] pcie: Fill PCIESlot link fields to support higher speeds and widths

[Alex Williamson]

Make use of the PCIESlot speed and width fields to update link
information beyond those configured in pcie_cap_v1_fill().  This is
only called for devices supporting a version 2 capability and
automatically skips any non-PCIESlot devices.  Only devices with
increased link values generate any visible config space differences.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci/pcie.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 61b7b96c52cd..556ec19925b9 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -27,6 +27,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pcie_regs.h"
+#include "hw/pci/pcie_port.h"
 #include "qemu/range.h"
 
 //#define DEBUG_PCIE
@@ -87,6 +88,74 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
     pci_set_word(cmask + PCI_EXP_LNKSTA, 0);
 }
 
+static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
+{
+    PCIESlot *s = (PCIESlot *)object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT);
+    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
+
+    /* Skip anything that isn't a PCIESlot */
+    if (!s) {
+        return;
+    }
+
+    /* Clear and fill LNKCAP from what was configured above */
+    pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP,
+                                 PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);
+    pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
+                               QEMU_PCI_EXP_LNKCAP_MLW(s->width) |
+                               QEMU_PCI_EXP_LNKCAP_MLS(s->speed));
+
+    /*
+     * Link bandwidth notification is required for all root ports and
+     * downstream ports supporting links wider than x1.
+     */
+    if (s->width > QEMU_PCI_EXP_LNK_X1) {
+        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
+                                   PCI_EXP_LNKCAP_LBNC);
+    }
+
+    if (s->speed > QEMU_PCI_EXP_LNK_2_5GT) {
+        /*
+         * Hot-plug capable downstream ports and downstream ports supporting
+         * link speeds greater than 5GT/s must hardwire PCI_EXP_LNKCAP_DLLLARC
+         * to 1b.  PCI_EXP_LNKCAP_DLLLARC implies PCI_EXP_LNKSTA_DLLLA, which
+         * we also hardwire to 1b here.  2.5GT/s hot-plug slots should also
+         * technically implement this, but it's not done here for compatibility.
+         */
+        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
+                                   PCI_EXP_LNKCAP_DLLLARC);
+        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
+                                   PCI_EXP_LNKSTA_DLLLA);
+
+        /*
+         * Target Link Speed defaults to the highest link speed supported by
+         * the component.  2.5GT/s devices are permitted to hardwire to zero.
+         */
+        pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKCTL2,
+                                     PCI_EXP_LNKCTL2_TLS);
+        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKCTL2,
+                                   QEMU_PCI_EXP_LNKCAP_MLS(s->speed) &
+                                   PCI_EXP_LNKCTL2_TLS);
+    }
+
+    /*
+     * 2.5 & 5.0GT/s can be fully described by LNKCAP, but 8.0GT/s is
+     * actually a reference to the highest bit supported in this register.
+     * We assume the device supports all link speeds.
+     */
+    if (s->speed > QEMU_PCI_EXP_LNK_5GT) {
+        pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP2, ~0U);
+        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
+                                   PCI_EXP_LNKCAP2_SLS_2_5GB |
+                                   PCI_EXP_LNKCAP2_SLS_5_0GB |
+                                   PCI_EXP_LNKCAP2_SLS_8_0GB);
+        if (s->speed > QEMU_PCI_EXP_LNK_8GT) {
+            pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
+                                       PCI_EXP_LNKCAP2_SLS_16_0GB);
+        }
+    }
+}
+
 int pcie_cap_init(PCIDevice *dev, uint8_t offset,
                   uint8_t type, uint8_t port,
                   Error **errp)
@@ -108,6 +177,9 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
     /* Filling values common with v1 */
     pcie_cap_v1_fill(dev, port, type, PCI_EXP_FLAGS_VER2);
 
+    /* Fill link speed and width options */
+    pcie_cap_fill_slot_lnk(dev);
+
     /* Filling v2 specific values */
     pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
                  PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);

Re: [Qemu-devel] [for-4.0 PATCH v3 5/9] pcie: Fill PCIESlot link fields to support higher speeds and widths

[Auger Eric]

Hi Alex,

On 12/4/18 5:26 PM, Alex Williamson wrote:
> Make use of the PCIESlot speed and width fields to update link
> information beyond those configured in pcie_cap_v1_fill().  This is
> only called for devices supporting a version 2 capability and
> automatically skips any non-PCIESlot devices.  Only devices with
> increased link values generate any visible config space differences.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/pci/pcie.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 61b7b96c52cd..556ec19925b9 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -27,6 +27,7 @@
>  #include "hw/pci/msi.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci/pcie_regs.h"
> +#include "hw/pci/pcie_port.h"
>  #include "qemu/range.h"
>  
>  //#define DEBUG_PCIE
> @@ -87,6 +88,74 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
>      pci_set_word(cmask + PCI_EXP_LNKSTA, 0);
>  }
>  
> +static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
> +{
> +    PCIESlot *s = (PCIESlot *)object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT);
> +    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
> +
> +    /* Skip anything that isn't a PCIESlot */
> +    if (!s) {
> +        return;
> +    }
> +
> +    /* Clear and fill LNKCAP from what was configured above */
> +    pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP,
> +                                 PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);
> +    pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> +                               QEMU_PCI_EXP_LNKCAP_MLW(s->width) |
> +                               QEMU_PCI_EXP_LNKCAP_MLS(s->speed));
> +
> +    /*
> +     * Link bandwidth notification is required for all root ports and
> +     * downstream ports supporting links wider than x1.


> +     */
> +    if (s->width > QEMU_PCI_EXP_LNK_X1) {
> +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> +                                   PCI_EXP_LNKCAP_LBNC);
spec also says "This capability is required for all Root
Ports and Switch Downstream Ports supporting Links wider than
x1 and/or multiple Link speeds."

I see below you are likely to report several speed vectors if speed >
5GT/s so you may also add a test on the speed.

How are the device types checked?
> +    }
> +
> +    if (s->speed > QEMU_PCI_EXP_LNK_2_5GT) {
> +        /*
> +         * Hot-plug capable downstream ports and downstream ports supporting
> +         * link speeds greater than 5GT/s must hardwire PCI_EXP_LNKCAP_DLLLARC
> +         * to 1b.  PCI_EXP_LNKCAP_DLLLARC implies PCI_EXP_LNKSTA_DLLLA, which
> +         * we also hardwire to 1b here.  2.5GT/s hot-plug slots should also
> +         * technically implement this, but it's not done here for compatibility.
> +         */
> +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> +                                   PCI_EXP_LNKCAP_DLLLARC);
> +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
> +                                   PCI_EXP_LNKSTA_DLLLA);
> +
> +        /*
> +         * Target Link Speed defaults to the highest link speed supported by
> +         * the component.  2.5GT/s devices are permitted to hardwire to zero.
> +         */
> +        pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKCTL2,
> +                                     PCI_EXP_LNKCTL2_TLS);
> +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKCTL2,
> +                                   QEMU_PCI_EXP_LNKCAP_MLS(s->speed) &
> +                                   PCI_EXP_LNKCTL2_TLS);
> +    }
> +
> +    /*
> +     * 2.5 & 5.0GT/s can be fully described by LNKCAP, but 8.0GT/s is
> +     * actually a reference to the highest bit supported in this register.
> +     * We assume the device supports all link speeds.
> +     */
> +    if (s->speed > QEMU_PCI_EXP_LNK_5GT) {
> +        pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP2, ~0U);
> +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
> +                                   PCI_EXP_LNKCAP2_SLS_2_5GB |
> +                                   PCI_EXP_LNKCAP2_SLS_5_0GB |
> +                                   PCI_EXP_LNKCAP2_SLS_8_0GB);
> +        if (s->speed > QEMU_PCI_EXP_LNK_8GT) {
> +            pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
> +                                       PCI_EXP_LNKCAP2_SLS_16_0GB);
I fail to understand why for 5GB you don't see both 2.5 and 5 as well.
> +        }
> +    }
> +}
> +
>  int pcie_cap_init(PCIDevice *dev, uint8_t offset,
>                    uint8_t type, uint8_t port,
>                    Error **errp)
> @@ -108,6 +177,9 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
>      /* Filling values common with v1 */
>      pcie_cap_v1_fill(dev, port, type, PCI_EXP_FLAGS_VER2);
>  
> +    /* Fill link speed and width options */
> +    pcie_cap_fill_slot_lnk(dev);
> +
>      /* Filling v2 specific values */
>      pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
>                   PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
> 
> 

Thanks

Eric

Re: [Qemu-devel] [for-4.0 PATCH v3 5/9] pcie: Fill PCIESlot link fields to support higher speeds and widths

[Alex Williamson]

On Thu, 6 Dec 2018 12:08:09 +0100
Auger Eric <eric.auger@redhat.com> wrote:

> Hi Alex,
> 
> On 12/4/18 5:26 PM, Alex Williamson wrote:
> > Make use of the PCIESlot speed and width fields to update link
> > information beyond those configured in pcie_cap_v1_fill().  This is
> > only called for devices supporting a version 2 capability and
> > automatically skips any non-PCIESlot devices.  Only devices with
> > increased link values generate any visible config space differences.
> > 
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Tested-by: Geoffrey McRae <geoff@hostfission.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  hw/pci/pcie.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index 61b7b96c52cd..556ec19925b9 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -27,6 +27,7 @@
> >  #include "hw/pci/msi.h"
> >  #include "hw/pci/pci_bus.h"
> >  #include "hw/pci/pcie_regs.h"
> > +#include "hw/pci/pcie_port.h"
> >  #include "qemu/range.h"
> >  
> >  //#define DEBUG_PCIE
> > @@ -87,6 +88,74 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
> >      pci_set_word(cmask + PCI_EXP_LNKSTA, 0);
> >  }
> >  
> > +static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
> > +{
> > +    PCIESlot *s = (PCIESlot *)object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT);
> > +    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
> > +
> > +    /* Skip anything that isn't a PCIESlot */
> > +    if (!s) {
> > +        return;
> > +    }
> > +
> > +    /* Clear and fill LNKCAP from what was configured above */
> > +    pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP,
> > +                                 PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);
> > +    pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> > +                               QEMU_PCI_EXP_LNKCAP_MLW(s->width) |
> > +                               QEMU_PCI_EXP_LNKCAP_MLS(s->speed));
> > +
> > +    /*
> > +     * Link bandwidth notification is required for all root ports and
> > +     * downstream ports supporting links wider than x1.  
> 
> 
> > +     */
> > +    if (s->width > QEMU_PCI_EXP_LNK_X1) {
> > +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> > +                                   PCI_EXP_LNKCAP_LBNC);  
> spec also says "This capability is required for all Root
> Ports and Switch Downstream Ports supporting Links wider than
> x1 and/or multiple Link speeds."
> 
> I see below you are likely to report several speed vectors if speed >
> 5GT/s so you may also add a test on the speed.

Good catch, so I'll change the above test to:

    if (s->width > QEMU_PCI_EXP_LNK_X1 ||
        s->speed > QEMU_PCI_EXP_LNK_2_5GT) {

> How are the device types checked?

Via the object_dynamic_cast() at the top of the function, we'll drop
anything that isn't a descendant of TYPE_PCIE_SLOT.

> > +    }
> > +
> > +    if (s->speed > QEMU_PCI_EXP_LNK_2_5GT) {
> > +        /*
> > +         * Hot-plug capable downstream ports and downstream ports supporting
> > +         * link speeds greater than 5GT/s must hardwire PCI_EXP_LNKCAP_DLLLARC
> > +         * to 1b.  PCI_EXP_LNKCAP_DLLLARC implies PCI_EXP_LNKSTA_DLLLA, which
> > +         * we also hardwire to 1b here.  2.5GT/s hot-plug slots should also
> > +         * technically implement this, but it's not done here for compatibility.
> > +         */
> > +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> > +                                   PCI_EXP_LNKCAP_DLLLARC);
> > +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
> > +                                   PCI_EXP_LNKSTA_DLLLA);
> > +
> > +        /*
> > +         * Target Link Speed defaults to the highest link speed supported by
> > +         * the component.  2.5GT/s devices are permitted to hardwire to zero.
> > +         */
> > +        pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKCTL2,
> > +                                     PCI_EXP_LNKCTL2_TLS);
> > +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKCTL2,
> > +                                   QEMU_PCI_EXP_LNKCAP_MLS(s->speed) &
> > +                                   PCI_EXP_LNKCTL2_TLS);
> > +    }
> > +
> > +    /*
> > +     * 2.5 & 5.0GT/s can be fully described by LNKCAP, but 8.0GT/s is
> > +     * actually a reference to the highest bit supported in this register.
> > +     * We assume the device supports all link speeds.
> > +     */
> > +    if (s->speed > QEMU_PCI_EXP_LNK_5GT) {
> > +        pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP2, ~0U);
> > +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
> > +                                   PCI_EXP_LNKCAP2_SLS_2_5GB |
> > +                                   PCI_EXP_LNKCAP2_SLS_5_0GB |
> > +                                   PCI_EXP_LNKCAP2_SLS_8_0GB);
> > +        if (s->speed > QEMU_PCI_EXP_LNK_8GT) {
> > +            pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
> > +                                       PCI_EXP_LNKCAP2_SLS_16_0GB);  
> I fail to understand why for 5GB you don't see both 2.5 and 5 as well.

Because there was a bit of a glitch in the PCIe 2.0 spec where lnkcap2
is listed as a placeholder, so a strictly gen2 device doesn't need to
implement lnkcap2.  In gen1, lnkcap supported link speeds (SLS) vector
defined 0001b for 2.5GT/s, gen2 came along and defined 0010b for
2.5GT/s AND 5GT/s, then in the 3.0 spec they decided to slightly
re-purpose this and SLS became MLS (max link speed), and introduced
lnkcap2 to indicate which speeds were supported.  So the original spec
definition of SLS didn't really give hardware the flexibility if they
should decide they don't want to validate intermediate link speeds.
Thanks,

Alex

> > +        }
> > +    }
> > +}
> > +
> >  int pcie_cap_init(PCIDevice *dev, uint8_t offset,
> >                    uint8_t type, uint8_t port,
> >                    Error **errp)
> > @@ -108,6 +177,9 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
> >      /* Filling values common with v1 */
> >      pcie_cap_v1_fill(dev, port, type, PCI_EXP_FLAGS_VER2);
> >  
> > +    /* Fill link speed and width options */
> > +    pcie_cap_fill_slot_lnk(dev);
> > +
> >      /* Filling v2 specific values */
> >      pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
> >                   PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
> > 
> >   
> 
> Thanks
> 
> Eric

Re: [Qemu-devel] [for-4.0 PATCH v3 5/9] pcie: Fill PCIESlot link fields to support higher speeds and widths

[Auger Eric]

Hi Alex,

On 12/6/18 5:00 PM, Alex Williamson wrote:
> On Thu, 6 Dec 2018 12:08:09 +0100
> Auger Eric <eric.auger@redhat.com> wrote:
> 
>> Hi Alex,
>>
>> On 12/4/18 5:26 PM, Alex Williamson wrote:
>>> Make use of the PCIESlot speed and width fields to update link
>>> information beyond those configured in pcie_cap_v1_fill().  This is
>>> only called for devices supporting a version 2 capability and
>>> automatically skips any non-PCIESlot devices.  Only devices with
>>> increased link values generate any visible config space differences.
>>>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>> Tested-by: Geoffrey McRae <geoff@hostfission.com>
>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>>> ---
>>>  hw/pci/pcie.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 72 insertions(+)
>>>
>>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>>> index 61b7b96c52cd..556ec19925b9 100644
>>> --- a/hw/pci/pcie.c
>>> +++ b/hw/pci/pcie.c
>>> @@ -27,6 +27,7 @@
>>>  #include "hw/pci/msi.h"
>>>  #include "hw/pci/pci_bus.h"
>>>  #include "hw/pci/pcie_regs.h"
>>> +#include "hw/pci/pcie_port.h"
>>>  #include "qemu/range.h"
>>>  
>>>  //#define DEBUG_PCIE
>>> @@ -87,6 +88,74 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
>>>      pci_set_word(cmask + PCI_EXP_LNKSTA, 0);
>>>  }
>>>  
>>> +static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
>>> +{
>>> +    PCIESlot *s = (PCIESlot *)object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT);
>>> +    uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
>>> +
>>> +    /* Skip anything that isn't a PCIESlot */
>>> +    if (!s) {
>>> +        return;
>>> +    }
>>> +
>>> +    /* Clear and fill LNKCAP from what was configured above */
>>> +    pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP,
>>> +                                 PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);
>>> +    pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
>>> +                               QEMU_PCI_EXP_LNKCAP_MLW(s->width) |
>>> +                               QEMU_PCI_EXP_LNKCAP_MLS(s->speed));
>>> +
>>> +    /*
>>> +     * Link bandwidth notification is required for all root ports and
>>> +     * downstream ports supporting links wider than x1.  
>>
>>
>>> +     */
>>> +    if (s->width > QEMU_PCI_EXP_LNK_X1) {
>>> +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
>>> +                                   PCI_EXP_LNKCAP_LBNC);  
>> spec also says "This capability is required for all Root
>> Ports and Switch Downstream Ports supporting Links wider than
>> x1 and/or multiple Link speeds."
>>
>> I see below you are likely to report several speed vectors if speed >
>> 5GT/s so you may also add a test on the speed.
> 
> Good catch, so I'll change the above test to:
> 
>     if (s->width > QEMU_PCI_EXP_LNK_X1 ||
>         s->speed > QEMU_PCI_EXP_LNK_2_5GT) {
> 
>> How are the device types checked?
> 
> Via the object_dynamic_cast() at the top of the function, we'll drop
> anything that isn't a descendant of TYPE_PCIE_SLOT.

Ah OK
> 
>>> +    }
>>> +
>>> +    if (s->speed > QEMU_PCI_EXP_LNK_2_5GT) {
>>> +        /*
>>> +         * Hot-plug capable downstream ports and downstream ports supporting
>>> +         * link speeds greater than 5GT/s must hardwire PCI_EXP_LNKCAP_DLLLARC
>>> +         * to 1b.  PCI_EXP_LNKCAP_DLLLARC implies PCI_EXP_LNKSTA_DLLLA, which
>>> +         * we also hardwire to 1b here.  2.5GT/s hot-plug slots should also
>>> +         * technically implement this, but it's not done here for compatibility.
>>> +         */
>>> +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
>>> +                                   PCI_EXP_LNKCAP_DLLLARC);
>>> +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
>>> +                                   PCI_EXP_LNKSTA_DLLLA);
>>> +
>>> +        /*
>>> +         * Target Link Speed defaults to the highest link speed supported by
>>> +         * the component.  2.5GT/s devices are permitted to hardwire to zero.
>>> +         */
>>> +        pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKCTL2,
>>> +                                     PCI_EXP_LNKCTL2_TLS);
>>> +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKCTL2,
>>> +                                   QEMU_PCI_EXP_LNKCAP_MLS(s->speed) &
>>> +                                   PCI_EXP_LNKCTL2_TLS);
>>> +    }
>>> +
>>> +    /*
>>> +     * 2.5 & 5.0GT/s can be fully described by LNKCAP, but 8.0GT/s is
>>> +     * actually a reference to the highest bit supported in this register.
>>> +     * We assume the device supports all link speeds.
>>> +     */
>>> +    if (s->speed > QEMU_PCI_EXP_LNK_5GT) {
>>> +        pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP2, ~0U);
>>> +        pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
>>> +                                   PCI_EXP_LNKCAP2_SLS_2_5GB |
>>> +                                   PCI_EXP_LNKCAP2_SLS_5_0GB |
>>> +                                   PCI_EXP_LNKCAP2_SLS_8_0GB);
>>> +        if (s->speed > QEMU_PCI_EXP_LNK_8GT) {
>>> +            pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP2,
>>> +                                       PCI_EXP_LNKCAP2_SLS_16_0GB);  
>> I fail to understand why for 5GB you don't see both 2.5 and 5 as well.
> 
> Because there was a bit of a glitch in the PCIe 2.0 spec where lnkcap2
> is listed as a placeholder, so a strictly gen2 device doesn't need to
> implement lnkcap2.  In gen1, lnkcap supported link speeds (SLS) vector
> defined 0001b for 2.5GT/s, gen2 came along and defined 0010b for
> 2.5GT/s AND 5GT/s, then in the 3.0 spec they decided to slightly
> re-purpose this and SLS became MLS (max link speed), and introduced
> lnkcap2 to indicate which speeds were supported.  So the original spec
> definition of SLS didn't really give hardware the flexibility if they
> should decide they don't want to validate intermediate link speeds.

OK Thanks

Eric
> Thanks,
> 
> Alex
> 
>>> +        }
>>> +    }
>>> +}
>>> +
>>>  int pcie_cap_init(PCIDevice *dev, uint8_t offset,
>>>                    uint8_t type, uint8_t port,
>>>                    Error **errp)
>>> @@ -108,6 +177,9 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
>>>      /* Filling values common with v1 */
>>>      pcie_cap_v1_fill(dev, port, type, PCI_EXP_FLAGS_VER2);
>>>  
>>> +    /* Fill link speed and width options */
>>> +    pcie_cap_fill_slot_lnk(dev);
>>> +
>>>      /* Filling v2 specific values */
>>>      pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
>>>                   PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
>>>
>>>   
>>
>> Thanks
>>
>> Eric
> 

[Qemu-devel] [for-4.0 PATCH v3 6/9] pcie: Allow generic PCIe root port to specify link speed and width

[Alex Williamson]

Allow users to experimentally specify speed and width values for the
generic PCIe root port.  Defaults remain at 2.5GT/s & x1 for
compatiblity with the intent to only support changing defaults via
machine types for now.

Note for libvirt testing that pcie-root-port controllers are given
default names like "pci.7" which don't play well with using the
"-set device.$name.$prop=$value" options accessible to us via
<qemu:commandline> options.  The solution is to add an <alias> to the
pcie-root-port <controller>, for example:

    <controller type='pci' index='7' model='pcie-root-port'>
      <model name='pcie-root-port'/>
      <target chassis='7' port='0x15'/>
      <alias name='ua-gfx0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x5'/>
    </controller>

The "ua-" here is a mandatory prefix.  We can then use:

  <qemu:commandline>
    <qemu:arg value='-set'/>
    <qemu:arg value='device.ua-gfx0.x-speed=8'/>
    <qemu:arg value='-set'/>
    <qemu:arg value='device.ua-gfx0.x-width=16'/>
  </qemu:commandline>

or, without an alias, set globals such as:

  <qemu:commandline>
    <qemu:arg value='-global'/>
    <qemu:arg value='pcie-root-port.x-speed=8'/>
    <qemu:arg value='-global'/>
    <qemu:arg value='pcie-root-port.x-width=16'/>
  </qemu:commandline>

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci-bridge/gen_pcie_root_port.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index 299de429ec1e..ca5418a89dd2 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -124,6 +124,10 @@ static Property gen_rp_props[] = {
                      res_reserve.mem_pref_32, -1),
     DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort,
                      res_reserve.mem_pref_64, -1),
+    DEFINE_PROP_PCIE_LINK_SPEED("x-speed", PCIESlot,
+                                speed, PCIE_LINK_SPEED_2_5),
+    DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
+                                width, PCIE_LINK_WIDTH_1),
     DEFINE_PROP_END_OF_LIST()
 };
 

Re: [Qemu-devel] [for-4.0 PATCH v3 6/9] pcie: Allow generic PCIe root port to specify link speed and width

[Auger Eric]

Hi,

On 12/4/18 5:26 PM, Alex Williamson wrote:
> Allow users to experimentally specify speed and width values for the
> generic PCIe root port.  Defaults remain at 2.5GT/s & x1 for
> compatiblity with the intent to only support changing defaults via
> machine types for now.
> 
> Note for libvirt testing that pcie-root-port controllers are given
> default names like "pci.7" which don't play well with using the
> "-set device.$name.$prop=$value" options accessible to us via
> <qemu:commandline> options.  The solution is to add an <alias> to the
> pcie-root-port <controller>, for example:
> 
>     <controller type='pci' index='7' model='pcie-root-port'>
>       <model name='pcie-root-port'/>
>       <target chassis='7' port='0x15'/>
>       <alias name='ua-gfx0'/>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x5'/>
>     </controller>
> 
> The "ua-" here is a mandatory prefix.  We can then use:
> 
>   <qemu:commandline>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.ua-gfx0.x-speed=8'/>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.ua-gfx0.x-width=16'/>
>   </qemu:commandline>
> 
> or, without an alias, set globals such as:
> 
>   <qemu:commandline>
>     <qemu:arg value='-global'/>
>     <qemu:arg value='pcie-root-port.x-speed=8'/>
>     <qemu:arg value='-global'/>
>     <qemu:arg value='pcie-root-port.x-width=16'/>
>   </qemu:commandline>
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  hw/pci-bridge/gen_pcie_root_port.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index 299de429ec1e..ca5418a89dd2 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -124,6 +124,10 @@ static Property gen_rp_props[] = {
>                       res_reserve.mem_pref_32, -1),
>      DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort,
>                       res_reserve.mem_pref_64, -1),
> +    DEFINE_PROP_PCIE_LINK_SPEED("x-speed", PCIESlot,
> +                                speed, PCIE_LINK_SPEED_2_5),
> +    DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
> +                                width, PCIE_LINK_WIDTH_1),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> 
> 

[Qemu-devel] [for-4.0 PATCH v3 7/9] vfio/pci: Remove PCIe Link Status emulation

[Alex Williamson]

Now that the downstream port will virtually negotiate itself to the
link status of the downstream devie, we can remove this emulation.
It's not clear that it was every terribly useful anyway.

Tested-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 74f9a46b4be0..c0cb1ec28908 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1901,12 +1901,6 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
                            QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT), ~0);
             vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKCTL, 0, ~0);
         }
-
-        /* Mark the Link Status bits as emulated to allow virtual negotiation */
-        vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKSTA,
-                               pci_get_word(vdev->pdev.config + pos +
-                                            PCI_EXP_LNKSTA),
-                               PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);
     }
 
     /*

Re: [Qemu-devel] [for-4.0 PATCH v3 7/9] vfio/pci: Remove PCIe Link Status emulation

[Auger Eric]

Hi
On 12/4/18 5:27 PM, Alex Williamson wrote:
> Now that the downstream port will virtually negotiate itself to the
> link status of the downstream devie, we can remove this emulation.
s/devie/device
> It's not clear that it was every terribly useful anyway.
> 
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

> ---
>  hw/vfio/pci.c |    6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 74f9a46b4be0..c0cb1ec28908 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1901,12 +1901,6 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>                             QEMU_PCI_EXP_LNKCAP_MLS(QEMU_PCI_EXP_LNK_2_5GT), ~0);
>              vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKCTL, 0, ~0);
>          }
> -
> -        /* Mark the Link Status bits as emulated to allow virtual negotiation */
> -        vfio_add_emulated_word(vdev, pos + PCI_EXP_LNKSTA,
> -                               pci_get_word(vdev->pdev.config + pos +
> -                                            PCI_EXP_LNKSTA),
> -                               PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
>      }
>  
>      /*
> 
> 

[Qemu-devel] [for-4.0 PATCH v3 8/9] q35/440fx/arm/spapr: Add QEMU 4.0 machine type

[Alex Williamson]

Including all machine types that might have a pcie-root-port.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/arm/virt.c        |   19 +++++++++++++++++--
 hw/i386/pc_piix.c    |   15 ++++++++++++---
 hw/i386/pc_q35.c     |   13 +++++++++++--
 hw/ppc/spapr.c       |   25 ++++++++++++++++++++++---
 include/hw/compat.h  |    3 +++
 include/hw/i386/pc.h |    3 +++
 6 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f69e7eb39977..beaf6bc43905 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1797,7 +1797,7 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
-static void virt_3_1_instance_init(Object *obj)
+static void virt_4_0_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
@@ -1867,10 +1867,25 @@ static void virt_3_1_instance_init(Object *obj)
     vms->irqmap = a15irqmap;
 }
 
+static void virt_machine_4_0_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(4, 0)
+
+#define VIRT_COMPAT_3_1 \
+    HW_COMPAT_3_1
+
+static void virt_3_1_instance_init(Object *obj)
+{
+    virt_4_0_instance_init(obj);
+}
+
 static void virt_machine_3_1_options(MachineClass *mc)
 {
+    virt_machine_4_0_options(mc);
+    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_1);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
+DEFINE_VIRT_MACHINE(3, 1)
 
 #define VIRT_COMPAT_3_0 \
     HW_COMPAT_3_0
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7092d6d13f66..cfaa83ee2fad 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
 }
 
-static void pc_i440fx_3_1_machine_options(MachineClass *m)
+static void pc_i440fx_4_0_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->alias = "pc";
     m->is_default = 1;
 }
 
+DEFINE_I440FX_MACHINE(v4_0, "pc-i440fx-4.0", NULL,
+                      pc_i440fx_4_0_machine_options);
+
+static void pc_i440fx_3_1_machine_options(MachineClass *m)
+{
+    pc_i440fx_4_0_machine_options(m);
+    m->is_default = 0;
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
+}
+
 DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
                       pc_i440fx_3_1_machine_options);
 
 static void pc_i440fx_3_0_machine_options(MachineClass *m)
 {
     pc_i440fx_3_1_machine_options(m);
-    m->is_default = 0;
-    m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4702bb13c472..e245db096dc1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
     m->max_cpus = 288;
 }
 
-static void pc_q35_3_1_machine_options(MachineClass *m)
+static void pc_q35_4_0_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
+                   pc_q35_4_0_machine_options);
+
+static void pc_q35_3_1_machine_options(MachineClass *m)
+{
+    pc_q35_4_0_machine_options(m);
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
+}
+
 DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
                    pc_q35_3_1_machine_options);
 
 static void pc_q35_3_0_machine_options(MachineClass *m)
 {
     pc_q35_3_1_machine_options(m);
-    m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
 }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7afd1a175bf2..80d8498867a6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
     }                                                                \
     type_init(spapr_machine_register_##suffix)
 
- /*
+/*
+ * pseries-4.0
+ */
+static void spapr_machine_4_0_instance_options(MachineState *machine)
+{
+}
+
+static void spapr_machine_4_0_class_options(MachineClass *mc)
+{
+    /* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
+
+/*
  * pseries-3.1
  */
+#define SPAPR_COMPAT_3_1                                              \
+    HW_COMPAT_3_1
+
 static void spapr_machine_3_1_instance_options(MachineState *machine)
 {
+    spapr_machine_4_0_instance_options(machine);
 }
 
 static void spapr_machine_3_1_class_options(MachineClass *mc)
 {
-    /* Defaults for the latest behaviour inherited from the base class */
+    spapr_machine_3_1_class_options(mc);
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
 }
 
-DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
+DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
 
 /*
  * pseries-3.0
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 6f4d5fc64704..70958328fe7a 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_3_1 \
+    /* empty */
+
 #define HW_COMPAT_3_0 \
     /* empty */
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 136fe497b6b2..cb645bf368a3 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+#define PC_COMPAT_3_1 \
+    HW_COMPAT_3_1 \
+
 #define PC_COMPAT_3_0 \
     HW_COMPAT_3_0 \
     {\

[Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Alex Williamson]

For upcoming pcie-root-port compatibility, among possibly others.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Re-spinning just this patch to include machine bump for ccw and
Cc'ing s390/ccw maintainers.  I think this rounds out all the
versioned machines per Peter's request.  I can respin a full series,
but at the risk of confounding automated tools, I didn't want to
further bombard inboxes with a high cadence.  Thanks,

Alex

 hw/arm/virt.c              |   19 +++++++++++++++++--
 hw/i386/pc_piix.c          |   15 ++++++++++++---
 hw/i386/pc_q35.c           |   13 +++++++++++--
 hw/ppc/spapr.c             |   25 ++++++++++++++++++++++---
 hw/s390x/s390-virtio-ccw.c |   17 ++++++++++++++++-
 include/hw/compat.h        |    3 +++
 include/hw/i386/pc.h       |    3 +++
 7 files changed, 84 insertions(+), 11 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f69e7eb39977..beaf6bc43905 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1797,7 +1797,7 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
-static void virt_3_1_instance_init(Object *obj)
+static void virt_4_0_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
@@ -1867,10 +1867,25 @@ static void virt_3_1_instance_init(Object *obj)
     vms->irqmap = a15irqmap;
 }
 
+static void virt_machine_4_0_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(4, 0)
+
+#define VIRT_COMPAT_3_1 \
+    HW_COMPAT_3_1
+
+static void virt_3_1_instance_init(Object *obj)
+{
+    virt_4_0_instance_init(obj);
+}
+
 static void virt_machine_3_1_options(MachineClass *mc)
 {
+    virt_machine_4_0_options(mc);
+    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_1);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
+DEFINE_VIRT_MACHINE(3, 1)
 
 #define VIRT_COMPAT_3_0 \
     HW_COMPAT_3_0
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7092d6d13f66..cfaa83ee2fad 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
 }
 
-static void pc_i440fx_3_1_machine_options(MachineClass *m)
+static void pc_i440fx_4_0_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->alias = "pc";
     m->is_default = 1;
 }
 
+DEFINE_I440FX_MACHINE(v4_0, "pc-i440fx-4.0", NULL,
+                      pc_i440fx_4_0_machine_options);
+
+static void pc_i440fx_3_1_machine_options(MachineClass *m)
+{
+    pc_i440fx_4_0_machine_options(m);
+    m->is_default = 0;
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
+}
+
 DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
                       pc_i440fx_3_1_machine_options);
 
 static void pc_i440fx_3_0_machine_options(MachineClass *m)
 {
     pc_i440fx_3_1_machine_options(m);
-    m->is_default = 0;
-    m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4702bb13c472..e245db096dc1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
     m->max_cpus = 288;
 }
 
-static void pc_q35_3_1_machine_options(MachineClass *m)
+static void pc_q35_4_0_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
+                   pc_q35_4_0_machine_options);
+
+static void pc_q35_3_1_machine_options(MachineClass *m)
+{
+    pc_q35_4_0_machine_options(m);
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
+}
+
 DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
                    pc_q35_3_1_machine_options);
 
 static void pc_q35_3_0_machine_options(MachineClass *m)
 {
     pc_q35_3_1_machine_options(m);
-    m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
 }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7afd1a175bf2..80d8498867a6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
     }                                                                \
     type_init(spapr_machine_register_##suffix)
 
- /*
+/*
+ * pseries-4.0
+ */
+static void spapr_machine_4_0_instance_options(MachineState *machine)
+{
+}
+
+static void spapr_machine_4_0_class_options(MachineClass *mc)
+{
+    /* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
+
+/*
  * pseries-3.1
  */
+#define SPAPR_COMPAT_3_1                                              \
+    HW_COMPAT_3_1
+
 static void spapr_machine_3_1_instance_options(MachineState *machine)
 {
+    spapr_machine_4_0_instance_options(machine);
 }
 
 static void spapr_machine_3_1_class_options(MachineClass *mc)
 {
-    /* Defaults for the latest behaviour inherited from the base class */
+    spapr_machine_3_1_class_options(mc);
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
 }
 
-DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
+DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
 
 /*
  * pseries-3.0
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index a0615a8b35f5..fd9d0b0542bb 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -651,6 +651,9 @@ bool css_migration_enabled(void)
     }                                                                         \
     type_init(ccw_machine_register_##suffix)
 
+#define CCW_COMPAT_3_1 \
+        HW_COMPAT_3_1
+
 #define CCW_COMPAT_3_0 \
         HW_COMPAT_3_0
 
@@ -742,14 +745,26 @@ bool css_migration_enabled(void)
             .value    = "0",\
         },
 
+static void ccw_machine_4_0_instance_options(MachineState *machine)
+{
+}
+
+static void ccw_machine_4_0_class_options(MachineClass *mc)
+{
+}
+DEFINE_CCW_MACHINE(4_0, "4.0", true);
+
 static void ccw_machine_3_1_instance_options(MachineState *machine)
 {
+    ccw_machine_4_0_instance_options(machine);
 }
 
 static void ccw_machine_3_1_class_options(MachineClass *mc)
 {
+    ccw_machine_4_0_class_options(mc);
+    SET_MACHINE_COMPAT(mc, CCW_COMPAT_3_1);
 }
-DEFINE_CCW_MACHINE(3_1, "3.1", true);
+DEFINE_CCW_MACHINE(3_1, "3.1", false);
 
 static void ccw_machine_3_0_instance_options(MachineState *machine)
 {
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 6f4d5fc64704..70958328fe7a 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_3_1 \
+    /* empty */
+
 #define HW_COMPAT_3_0 \
     /* empty */
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 136fe497b6b2..cb645bf368a3 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+#define PC_COMPAT_3_1 \
+    HW_COMPAT_3_1 \
+
 #define PC_COMPAT_3_0 \
     HW_COMPAT_3_0 \
     {\

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Christian Borntraeger]

I think Conny has already added the s390/ccw part to her next tree. 
>From a quick glimpse both patches look identical.

On 04.12.2018 20:04, Alex Williamson wrote:
> For upcoming pcie-root-port compatibility, among possibly others.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> Re-spinning just this patch to include machine bump for ccw and
> Cc'ing s390/ccw maintainers.  I think this rounds out all the
> versioned machines per Peter's request.  I can respin a full series,
> but at the risk of confounding automated tools, I didn't want to
> further bombard inboxes with a high cadence.  Thanks,
> 
> Alex
> 
>  hw/arm/virt.c              |   19 +++++++++++++++++--
>  hw/i386/pc_piix.c          |   15 ++++++++++++---
>  hw/i386/pc_q35.c           |   13 +++++++++++--
>  hw/ppc/spapr.c             |   25 ++++++++++++++++++++++---
>  hw/s390x/s390-virtio-ccw.c |   17 ++++++++++++++++-
>  include/hw/compat.h        |    3 +++
>  include/hw/i386/pc.h       |    3 +++
>  7 files changed, 84 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f69e7eb39977..beaf6bc43905 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1797,7 +1797,7 @@ static void machvirt_machine_init(void)
>  }
>  type_init(machvirt_machine_init);
>  
> -static void virt_3_1_instance_init(Object *obj)
> +static void virt_4_0_instance_init(Object *obj)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> @@ -1867,10 +1867,25 @@ static void virt_3_1_instance_init(Object *obj)
>      vms->irqmap = a15irqmap;
>  }
>  
> +static void virt_machine_4_0_options(MachineClass *mc)
> +{
> +}
> +DEFINE_VIRT_MACHINE_AS_LATEST(4, 0)
> +
> +#define VIRT_COMPAT_3_1 \
> +    HW_COMPAT_3_1
> +
> +static void virt_3_1_instance_init(Object *obj)
> +{
> +    virt_4_0_instance_init(obj);
> +}
> +
>  static void virt_machine_3_1_options(MachineClass *mc)
>  {
> +    virt_machine_4_0_options(mc);
> +    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_1);
>  }
> -DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
> +DEFINE_VIRT_MACHINE(3, 1)
>  
>  #define VIRT_COMPAT_3_0 \
>      HW_COMPAT_3_0
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7092d6d13f66..cfaa83ee2fad 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
>      machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
>  }
>  
> -static void pc_i440fx_3_1_machine_options(MachineClass *m)
> +static void pc_i440fx_4_0_machine_options(MachineClass *m)
>  {
>      pc_i440fx_machine_options(m);
>      m->alias = "pc";
>      m->is_default = 1;
>  }
>  
> +DEFINE_I440FX_MACHINE(v4_0, "pc-i440fx-4.0", NULL,
> +                      pc_i440fx_4_0_machine_options);
> +
> +static void pc_i440fx_3_1_machine_options(MachineClass *m)
> +{
> +    pc_i440fx_4_0_machine_options(m);
> +    m->is_default = 0;
> +    m->alias = NULL;
> +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> +}
> +
>  DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
>                        pc_i440fx_3_1_machine_options);
>  
>  static void pc_i440fx_3_0_machine_options(MachineClass *m)
>  {
>      pc_i440fx_3_1_machine_options(m);
> -    m->is_default = 0;
> -    m->alias = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 4702bb13c472..e245db096dc1 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
>      m->max_cpus = 288;
>  }
>  
> -static void pc_q35_3_1_machine_options(MachineClass *m)
> +static void pc_q35_4_0_machine_options(MachineClass *m)
>  {
>      pc_q35_machine_options(m);
>      m->alias = "q35";
>  }
>  
> +DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
> +                   pc_q35_4_0_machine_options);
> +
> +static void pc_q35_3_1_machine_options(MachineClass *m)
> +{
> +    pc_q35_4_0_machine_options(m);
> +    m->alias = NULL;
> +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> +}
> +
>  DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
>                     pc_q35_3_1_machine_options);
>  
>  static void pc_q35_3_0_machine_options(MachineClass *m)
>  {
>      pc_q35_3_1_machine_options(m);
> -    m->alias = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
>  }
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7afd1a175bf2..80d8498867a6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
>      }                                                                \
>      type_init(spapr_machine_register_##suffix)
>  
> - /*
> +/*
> + * pseries-4.0
> + */
> +static void spapr_machine_4_0_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_4_0_class_options(MachineClass *mc)
> +{
> +    /* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> +
> +/*
>   * pseries-3.1
>   */
> +#define SPAPR_COMPAT_3_1                                              \
> +    HW_COMPAT_3_1
> +
>  static void spapr_machine_3_1_instance_options(MachineState *machine)
>  {
> +    spapr_machine_4_0_instance_options(machine);
>  }
>  
>  static void spapr_machine_3_1_class_options(MachineClass *mc)
>  {
> -    /* Defaults for the latest behaviour inherited from the base class */
> +    spapr_machine_3_1_class_options(mc);
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
>  }
>  
> -DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
> +DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
>  
>  /*
>   * pseries-3.0
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index a0615a8b35f5..fd9d0b0542bb 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -651,6 +651,9 @@ bool css_migration_enabled(void)
>      }                                                                         \
>      type_init(ccw_machine_register_##suffix)
>  
> +#define CCW_COMPAT_3_1 \
> +        HW_COMPAT_3_1
> +
>  #define CCW_COMPAT_3_0 \
>          HW_COMPAT_3_0
>  
> @@ -742,14 +745,26 @@ bool css_migration_enabled(void)
>              .value    = "0",\
>          },
>  
> +static void ccw_machine_4_0_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void ccw_machine_4_0_class_options(MachineClass *mc)
> +{
> +}
> +DEFINE_CCW_MACHINE(4_0, "4.0", true);
> +
>  static void ccw_machine_3_1_instance_options(MachineState *machine)
>  {
> +    ccw_machine_4_0_instance_options(machine);
>  }
>  
>  static void ccw_machine_3_1_class_options(MachineClass *mc)
>  {
> +    ccw_machine_4_0_class_options(mc);
> +    SET_MACHINE_COMPAT(mc, CCW_COMPAT_3_1);
>  }
> -DEFINE_CCW_MACHINE(3_1, "3.1", true);
> +DEFINE_CCW_MACHINE(3_1, "3.1", false);
>  
>  static void ccw_machine_3_0_instance_options(MachineState *machine)
>  {
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 6f4d5fc64704..70958328fe7a 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -1,6 +1,9 @@
>  #ifndef HW_COMPAT_H
>  #define HW_COMPAT_H
>  
> +#define HW_COMPAT_3_1 \
> +    /* empty */
> +
>  #define HW_COMPAT_3_0 \
>      /* empty */
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 136fe497b6b2..cb645bf368a3 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> +#define PC_COMPAT_3_1 \
> +    HW_COMPAT_3_1 \
> +
>  #define PC_COMPAT_3_0 \
>      HW_COMPAT_3_0 \
>      {\
> 

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Alex Williamson]

On Tue, 4 Dec 2018 20:16:44 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> I think Conny has already added the s390/ccw part to her next tree. 
> From a quick glimpse both patches look identical.

If so then we can just use the original v3 version of this patch that
touches all but ccw and let them come together in mainline.  My
assumption is that Peter is only trying to make sure all versioned
machines are updated early in this release, not necessarily that
they need to be updated together.  Thanks,

Alex
 
> On 04.12.2018 20:04, Alex Williamson wrote:
> > For upcoming pcie-root-port compatibility, among possibly others.
> > 
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> > Acked-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> > 
> > Re-spinning just this patch to include machine bump for ccw and
> > Cc'ing s390/ccw maintainers.  I think this rounds out all the
> > versioned machines per Peter's request.  I can respin a full series,
> > but at the risk of confounding automated tools, I didn't want to
> > further bombard inboxes with a high cadence.  Thanks,
> > 
> > Alex
> > 
> >  hw/arm/virt.c              |   19 +++++++++++++++++--
> >  hw/i386/pc_piix.c          |   15 ++++++++++++---
> >  hw/i386/pc_q35.c           |   13 +++++++++++--
> >  hw/ppc/spapr.c             |   25 ++++++++++++++++++++++---
> >  hw/s390x/s390-virtio-ccw.c |   17 ++++++++++++++++-
> >  include/hw/compat.h        |    3 +++
> >  include/hw/i386/pc.h       |    3 +++
> >  7 files changed, 84 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index f69e7eb39977..beaf6bc43905 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1797,7 +1797,7 @@ static void machvirt_machine_init(void)
> >  }
> >  type_init(machvirt_machine_init);
> >  
> > -static void virt_3_1_instance_init(Object *obj)
> > +static void virt_4_0_instance_init(Object *obj)
> >  {
> >      VirtMachineState *vms = VIRT_MACHINE(obj);
> >      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> > @@ -1867,10 +1867,25 @@ static void virt_3_1_instance_init(Object *obj)
> >      vms->irqmap = a15irqmap;
> >  }
> >  
> > +static void virt_machine_4_0_options(MachineClass *mc)
> > +{
> > +}
> > +DEFINE_VIRT_MACHINE_AS_LATEST(4, 0)
> > +
> > +#define VIRT_COMPAT_3_1 \
> > +    HW_COMPAT_3_1
> > +
> > +static void virt_3_1_instance_init(Object *obj)
> > +{
> > +    virt_4_0_instance_init(obj);
> > +}
> > +
> >  static void virt_machine_3_1_options(MachineClass *mc)
> >  {
> > +    virt_machine_4_0_options(mc);
> > +    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_1);
> >  }
> > -DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
> > +DEFINE_VIRT_MACHINE(3, 1)
> >  
> >  #define VIRT_COMPAT_3_0 \
> >      HW_COMPAT_3_0
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 7092d6d13f66..cfaa83ee2fad 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
> >      machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
> >  }
> >  
> > -static void pc_i440fx_3_1_machine_options(MachineClass *m)
> > +static void pc_i440fx_4_0_machine_options(MachineClass *m)
> >  {
> >      pc_i440fx_machine_options(m);
> >      m->alias = "pc";
> >      m->is_default = 1;
> >  }
> >  
> > +DEFINE_I440FX_MACHINE(v4_0, "pc-i440fx-4.0", NULL,
> > +                      pc_i440fx_4_0_machine_options);
> > +
> > +static void pc_i440fx_3_1_machine_options(MachineClass *m)
> > +{
> > +    pc_i440fx_4_0_machine_options(m);
> > +    m->is_default = 0;
> > +    m->alias = NULL;
> > +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> > +}
> > +
> >  DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
> >                        pc_i440fx_3_1_machine_options);
> >  
> >  static void pc_i440fx_3_0_machine_options(MachineClass *m)
> >  {
> >      pc_i440fx_3_1_machine_options(m);
> > -    m->is_default = 0;
> > -    m->alias = NULL;
> >      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
> >  }
> >  
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 4702bb13c472..e245db096dc1 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
> >      m->max_cpus = 288;
> >  }
> >  
> > -static void pc_q35_3_1_machine_options(MachineClass *m)
> > +static void pc_q35_4_0_machine_options(MachineClass *m)
> >  {
> >      pc_q35_machine_options(m);
> >      m->alias = "q35";
> >  }
> >  
> > +DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
> > +                   pc_q35_4_0_machine_options);
> > +
> > +static void pc_q35_3_1_machine_options(MachineClass *m)
> > +{
> > +    pc_q35_4_0_machine_options(m);
> > +    m->alias = NULL;
> > +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> > +}
> > +
> >  DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
> >                     pc_q35_3_1_machine_options);
> >  
> >  static void pc_q35_3_0_machine_options(MachineClass *m)
> >  {
> >      pc_q35_3_1_machine_options(m);
> > -    m->alias = NULL;
> >      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
> >  }
> >  
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7afd1a175bf2..80d8498867a6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
> >      }                                                                \
> >      type_init(spapr_machine_register_##suffix)
> >  
> > - /*
> > +/*
> > + * pseries-4.0
> > + */
> > +static void spapr_machine_4_0_instance_options(MachineState *machine)
> > +{
> > +}
> > +
> > +static void spapr_machine_4_0_class_options(MachineClass *mc)
> > +{
> > +    /* Defaults for the latest behaviour inherited from the base class */
> > +}
> > +
> > +DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> > +
> > +/*
> >   * pseries-3.1
> >   */
> > +#define SPAPR_COMPAT_3_1                                              \
> > +    HW_COMPAT_3_1
> > +
> >  static void spapr_machine_3_1_instance_options(MachineState *machine)
> >  {
> > +    spapr_machine_4_0_instance_options(machine);
> >  }
> >  
> >  static void spapr_machine_3_1_class_options(MachineClass *mc)
> >  {
> > -    /* Defaults for the latest behaviour inherited from the base class */
> > +    spapr_machine_3_1_class_options(mc);
> > +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> >  }
> >  
> > -DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
> > +DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> >  
> >  /*
> >   * pseries-3.0
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index a0615a8b35f5..fd9d0b0542bb 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -651,6 +651,9 @@ bool css_migration_enabled(void)
> >      }                                                                         \
> >      type_init(ccw_machine_register_##suffix)
> >  
> > +#define CCW_COMPAT_3_1 \
> > +        HW_COMPAT_3_1
> > +
> >  #define CCW_COMPAT_3_0 \
> >          HW_COMPAT_3_0
> >  
> > @@ -742,14 +745,26 @@ bool css_migration_enabled(void)
> >              .value    = "0",\
> >          },
> >  
> > +static void ccw_machine_4_0_instance_options(MachineState *machine)
> > +{
> > +}
> > +
> > +static void ccw_machine_4_0_class_options(MachineClass *mc)
> > +{
> > +}
> > +DEFINE_CCW_MACHINE(4_0, "4.0", true);
> > +
> >  static void ccw_machine_3_1_instance_options(MachineState *machine)
> >  {
> > +    ccw_machine_4_0_instance_options(machine);
> >  }
> >  
> >  static void ccw_machine_3_1_class_options(MachineClass *mc)
> >  {
> > +    ccw_machine_4_0_class_options(mc);
> > +    SET_MACHINE_COMPAT(mc, CCW_COMPAT_3_1);
> >  }
> > -DEFINE_CCW_MACHINE(3_1, "3.1", true);
> > +DEFINE_CCW_MACHINE(3_1, "3.1", false);
> >  
> >  static void ccw_machine_3_0_instance_options(MachineState *machine)
> >  {
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 6f4d5fc64704..70958328fe7a 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -1,6 +1,9 @@
> >  #ifndef HW_COMPAT_H
> >  #define HW_COMPAT_H
> >  
> > +#define HW_COMPAT_3_1 \
> > +    /* empty */
> > +
> >  #define HW_COMPAT_3_0 \
> >      /* empty */
> >  
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 136fe497b6b2..cb645bf368a3 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >  int e820_get_num_entries(void);
> >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >  
> > +#define PC_COMPAT_3_1 \
> > +    HW_COMPAT_3_1 \
> > +
> >  #define PC_COMPAT_3_0 \
> >      HW_COMPAT_3_0 \
> >      {\
> >   
> 

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Peter Maydell]

On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:
>
> On Tue, 4 Dec 2018 20:16:44 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
> > I think Conny has already added the s390/ccw part to her next tree.
> > From a quick glimpse both patches look identical.
>
> If so then we can just use the original v3 version of this patch that
> touches all but ccw and let them come together in mainline.  My
> assumption is that Peter is only trying to make sure all versioned
> machines are updated early in this release, not necessarily that
> they need to be updated together.

Yes, that's the idea. I also think it's a suboptimal idea
to include the version-number-bump patch in a series that's
adding some feature, because then if the feature itself
has to go through several rounds of patch review the
version-number-bump patch is stuck unapplied (we saw that
at the end of the 3.1 cycle), or it gets bumped by some
other unrelated series and then there's a merge conflict.
But that's more of a things-for-next time remark, no need
to rearrange this now.

thanks
-- PMM

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Alex Williamson]

On Tue, 4 Dec 2018 19:29:25 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:
> >
> > On Tue, 4 Dec 2018 20:16:44 +0100
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >  
> > > I think Conny has already added the s390/ccw part to her next tree.
> > > From a quick glimpse both patches look identical.  
> >
> > If so then we can just use the original v3 version of this patch that
> > touches all but ccw and let them come together in mainline.  My
> > assumption is that Peter is only trying to make sure all versioned
> > machines are updated early in this release, not necessarily that
> > they need to be updated together.  
> 
> Yes, that's the idea. I also think it's a suboptimal idea
> to include the version-number-bump patch in a series that's
> adding some feature, because then if the feature itself
> has to go through several rounds of patch review the
> version-number-bump patch is stuck unapplied (we saw that
> at the end of the 3.1 cycle), or it gets bumped by some
> other unrelated series and then there's a merge conflict.
> But that's more of a things-for-next time remark, no need
> to rearrange this now.

If you and the other stakeholders agree, you are more than welcome to
pluck this patch from the series and apply it as soon as 4.0 opens.  It
might make things a tiny bit easier down the road to avoid the
conflicts since we seem to have multiple contenders vying for this
update.  Maybe the best practice going forward is to open the merge
window with such a commit.  Thanks,

Alex

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Christian Borntraeger]

On 04.12.2018 20:56, Alex Williamson wrote:
> On Tue, 4 Dec 2018 19:29:25 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
>> On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:
>>>
>>> On Tue, 4 Dec 2018 20:16:44 +0100
>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>  
>>>> I think Conny has already added the s390/ccw part to her next tree.
>>>> From a quick glimpse both patches look identical.  
>>>
>>> If so then we can just use the original v3 version of this patch that
>>> touches all but ccw and let them come together in mainline.  My
>>> assumption is that Peter is only trying to make sure all versioned
>>> machines are updated early in this release, not necessarily that
>>> they need to be updated together.  
>>
>> Yes, that's the idea. I also think it's a suboptimal idea
>> to include the version-number-bump patch in a series that's
>> adding some feature, because then if the feature itself
>> has to go through several rounds of patch review the
>> version-number-bump patch is stuck unapplied (we saw that
>> at the end of the 3.1 cycle), or it gets bumped by some
>> other unrelated series and then there's a merge conflict.
>> But that's more of a things-for-next time remark, no need
>> to rearrange this now.
> 
> If you and the other stakeholders agree, you are more than welcome to
> pluck this patch from the series and apply it as soon as 4.0 opens.  It
> might make things a tiny bit easier down the road to avoid the
> conflicts since we seem to have multiple contenders vying for this
> update.  Maybe the best practice going forward is to open the merge
> window with such a commit.  Thanks,

I agree. Something like this should be the first commit after each release.

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Cornelia Huck]

On Tue, 4 Dec 2018 12:56:21 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Tue, 4 Dec 2018 19:29:25 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> > On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:  
> > >
> > > On Tue, 4 Dec 2018 20:16:44 +0100
> > > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > >    
> > > > I think Conny has already added the s390/ccw part to her next tree.
> > > > From a quick glimpse both patches look identical.    
> > >
> > > If so then we can just use the original v3 version of this patch that
> > > touches all but ccw and let them come together in mainline.  My
> > > assumption is that Peter is only trying to make sure all versioned
> > > machines are updated early in this release, not necessarily that
> > > they need to be updated together.    
> > 
> > Yes, that's the idea. I also think it's a suboptimal idea
> > to include the version-number-bump patch in a series that's
> > adding some feature, because then if the feature itself
> > has to go through several rounds of patch review the
> > version-number-bump patch is stuck unapplied (we saw that
> > at the end of the 3.1 cycle), or it gets bumped by some
> > other unrelated series and then there's a merge conflict.
> > But that's more of a things-for-next time remark, no need
> > to rearrange this now.  
> 
> If you and the other stakeholders agree, you are more than welcome to
> pluck this patch from the series and apply it as soon as 4.0 opens.  It
> might make things a tiny bit easier down the road to avoid the
> conflicts since we seem to have multiple contenders vying for this
> update.  Maybe the best practice going forward is to open the merge
> window with such a commit.  Thanks,

FWIW, I had planned to send a pull request with what is in my queue
(including the new machine type) first thing after 4.0 opens.

For the next release: Should we always create a patch like this that
adds the new type for all machines and queue this as the first thing
when the tree opens again? (I'd even be willing to do that...) For this
release, I would prefer to use the already-existing patches instead.

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Alex Williamson]

On Wed, 5 Dec 2018 09:32:21 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 4 Dec 2018 12:56:21 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > On Tue, 4 Dec 2018 19:29:25 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >   
> > > On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:    
> > > >
> > > > On Tue, 4 Dec 2018 20:16:44 +0100
> > > > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > > >      
> > > > > I think Conny has already added the s390/ccw part to her next tree.
> > > > > From a quick glimpse both patches look identical.      
> > > >
> > > > If so then we can just use the original v3 version of this patch that
> > > > touches all but ccw and let them come together in mainline.  My
> > > > assumption is that Peter is only trying to make sure all versioned
> > > > machines are updated early in this release, not necessarily that
> > > > they need to be updated together.      
> > > 
> > > Yes, that's the idea. I also think it's a suboptimal idea
> > > to include the version-number-bump patch in a series that's
> > > adding some feature, because then if the feature itself
> > > has to go through several rounds of patch review the
> > > version-number-bump patch is stuck unapplied (we saw that
> > > at the end of the 3.1 cycle), or it gets bumped by some
> > > other unrelated series and then there's a merge conflict.
> > > But that's more of a things-for-next time remark, no need
> > > to rearrange this now.    
> > 
> > If you and the other stakeholders agree, you are more than welcome to
> > pluck this patch from the series and apply it as soon as 4.0 opens.  It
> > might make things a tiny bit easier down the road to avoid the
> > conflicts since we seem to have multiple contenders vying for this
> > update.  Maybe the best practice going forward is to open the merge
> > window with such a commit.  Thanks,  
> 
> FWIW, I had planned to send a pull request with what is in my queue
> (including the new machine type) first thing after 4.0 opens.
> 
> For the next release: Should we always create a patch like this that
> adds the new type for all machines and queue this as the first thing
> when the tree opens again? (I'd even be willing to do that...) For this
> release, I would prefer to use the already-existing patches instead.

Ok, so we'll stick with the original v3 version that didn't include ccw
and Marc-André's series and this one can fight it out for the rest of
the versioned machines.  Please disregard this v3.1 patch including
ccw.  Thanks,

Alex

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Cornelia Huck]

On Wed, 5 Dec 2018 08:42:40 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Wed, 5 Dec 2018 09:32:21 +0100
> Cornelia Huck <cohuck@redhat.com> wrote:

> > For the next release: Should we always create a patch like this that
> > adds the new type for all machines and queue this as the first thing
> > when the tree opens again? (I'd even be willing to do that...) For this
> > release, I would prefer to use the already-existing patches instead.  
> 
> Ok, so we'll stick with the original v3 version that didn't include ccw
> and Marc-André's series and this one can fight it out for the rest of
> the versioned machines.  Please disregard this v3.1 patch including
> ccw.  Thanks,
> 
> Alex

Ok, I'll go ahead with my patch, then.

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Eduardo Habkost]

On Tue, Dec 04, 2018 at 12:56:21PM -0700, Alex Williamson wrote:
> On Tue, 4 Dec 2018 19:29:25 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> > On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:
> > >
> > > On Tue, 4 Dec 2018 20:16:44 +0100
> > > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > >  
> > > > I think Conny has already added the s390/ccw part to her next tree.
> > > > From a quick glimpse both patches look identical.  
> > >
> > > If so then we can just use the original v3 version of this patch that
> > > touches all but ccw and let them come together in mainline.  My
> > > assumption is that Peter is only trying to make sure all versioned
> > > machines are updated early in this release, not necessarily that
> > > they need to be updated together.  
> > 
> > Yes, that's the idea. I also think it's a suboptimal idea
> > to include the version-number-bump patch in a series that's
> > adding some feature, because then if the feature itself
> > has to go through several rounds of patch review the
> > version-number-bump patch is stuck unapplied (we saw that
> > at the end of the 3.1 cycle), or it gets bumped by some
> > other unrelated series and then there's a merge conflict.
> > But that's more of a things-for-next time remark, no need
> > to rearrange this now.
> 
> If you and the other stakeholders agree, you are more than welcome to
> pluck this patch from the series and apply it as soon as 4.0 opens.  It
> might make things a tiny bit easier down the road to avoid the
> conflicts since we seem to have multiple contenders vying for this
> update.  Maybe the best practice going forward is to open the merge
> window with such a commit.  Thanks,

I can queue v3 on machine-next and send a pull request as soon as
v3.1.0 is tagged.  Any objections?

-- 
Eduardo

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Alex Williamson]

On Thu, 6 Dec 2018 10:52:25 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Dec 04, 2018 at 12:56:21PM -0700, Alex Williamson wrote:
> > On Tue, 4 Dec 2018 19:29:25 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >   
> > > On Tue, 4 Dec 2018 at 19:26, Alex Williamson <alex.williamson@redhat.com> wrote:  
> > > >
> > > > On Tue, 4 Dec 2018 20:16:44 +0100
> > > > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > > >    
> > > > > I think Conny has already added the s390/ccw part to her next tree.
> > > > > From a quick glimpse both patches look identical.    
> > > >
> > > > If so then we can just use the original v3 version of this patch that
> > > > touches all but ccw and let them come together in mainline.  My
> > > > assumption is that Peter is only trying to make sure all versioned
> > > > machines are updated early in this release, not necessarily that
> > > > they need to be updated together.    
> > > 
> > > Yes, that's the idea. I also think it's a suboptimal idea
> > > to include the version-number-bump patch in a series that's
> > > adding some feature, because then if the feature itself
> > > has to go through several rounds of patch review the
> > > version-number-bump patch is stuck unapplied (we saw that
> > > at the end of the 3.1 cycle), or it gets bumped by some
> > > other unrelated series and then there's a merge conflict.
> > > But that's more of a things-for-next time remark, no need
> > > to rearrange this now.  
> > 
> > If you and the other stakeholders agree, you are more than welcome to
> > pluck this patch from the series and apply it as soon as 4.0 opens.  It
> > might make things a tiny bit easier down the road to avoid the
> > conflicts since we seem to have multiple contenders vying for this
> > update.  Maybe the best practice going forward is to open the merge
> > window with such a commit.  Thanks,  
> 
> I can queue v3 on machine-next and send a pull request as soon as
> v3.1.0 is tagged.  Any objections?

No objection from me, so long as the pull doesn't get delayed.  Please
also collect the Reviewed-by from Eric.  Thanks,

Alex

Re: [Qemu-devel] [for-4.0 PATCH v3.1 8/9] q35/440fx/arm/spapr/ccw: Add QEMU 4.0 machine type

[Marc-André Lureau]

Hi
On Tue, Dec 4, 2018 at 11:17 PM Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
>
> I think Conny has already added the s390/ccw part to her next tree.
> From a quick glimpse both patches look identical.
>

I also have them in my machine compatibilities series:
https://patchew.org/QEMU/20181204142023.15982-1-marcandre.lureau@redhat.com/20181204142023.15982-18-marcandre.lureau@redhat.com/
https://patchew.org/QEMU/20181204142023.15982-1-marcandre.lureau@redhat.com/20181204142023.15982-19-marcandre.lureau@redhat.com/

Whoever goes first :)

> On 04.12.2018 20:04, Alex Williamson wrote:
> > For upcoming pcie-root-port compatibility, among possibly others.
> >
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Cornelia Huck <cohuck@redhat.com>
> > Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> > Acked-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >
> > Re-spinning just this patch to include machine bump for ccw and
> > Cc'ing s390/ccw maintainers.  I think this rounds out all the
> > versioned machines per Peter's request.  I can respin a full series,
> > but at the risk of confounding automated tools, I didn't want to
> > further bombard inboxes with a high cadence.  Thanks,
> >
> > Alex
> >
> >  hw/arm/virt.c              |   19 +++++++++++++++++--
> >  hw/i386/pc_piix.c          |   15 ++++++++++++---
> >  hw/i386/pc_q35.c           |   13 +++++++++++--
> >  hw/ppc/spapr.c             |   25 ++++++++++++++++++++++---
> >  hw/s390x/s390-virtio-ccw.c |   17 ++++++++++++++++-
> >  include/hw/compat.h        |    3 +++
> >  include/hw/i386/pc.h       |    3 +++
> >  7 files changed, 84 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index f69e7eb39977..beaf6bc43905 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1797,7 +1797,7 @@ static void machvirt_machine_init(void)
> >  }
> >  type_init(machvirt_machine_init);
> >
> > -static void virt_3_1_instance_init(Object *obj)
> > +static void virt_4_0_instance_init(Object *obj)
> >  {
> >      VirtMachineState *vms = VIRT_MACHINE(obj);
> >      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> > @@ -1867,10 +1867,25 @@ static void virt_3_1_instance_init(Object *obj)
> >      vms->irqmap = a15irqmap;
> >  }
> >
> > +static void virt_machine_4_0_options(MachineClass *mc)
> > +{
> > +}
> > +DEFINE_VIRT_MACHINE_AS_LATEST(4, 0)
> > +
> > +#define VIRT_COMPAT_3_1 \
> > +    HW_COMPAT_3_1
> > +
> > +static void virt_3_1_instance_init(Object *obj)
> > +{
> > +    virt_4_0_instance_init(obj);
> > +}
> > +
> >  static void virt_machine_3_1_options(MachineClass *mc)
> >  {
> > +    virt_machine_4_0_options(mc);
> > +    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_1);
> >  }
> > -DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
> > +DEFINE_VIRT_MACHINE(3, 1)
> >
> >  #define VIRT_COMPAT_3_0 \
> >      HW_COMPAT_3_0
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 7092d6d13f66..cfaa83ee2fad 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
> >      machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
> >  }
> >
> > -static void pc_i440fx_3_1_machine_options(MachineClass *m)
> > +static void pc_i440fx_4_0_machine_options(MachineClass *m)
> >  {
> >      pc_i440fx_machine_options(m);
> >      m->alias = "pc";
> >      m->is_default = 1;
> >  }
> >
> > +DEFINE_I440FX_MACHINE(v4_0, "pc-i440fx-4.0", NULL,
> > +                      pc_i440fx_4_0_machine_options);
> > +
> > +static void pc_i440fx_3_1_machine_options(MachineClass *m)
> > +{
> > +    pc_i440fx_4_0_machine_options(m);
> > +    m->is_default = 0;
> > +    m->alias = NULL;
> > +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> > +}
> > +
> >  DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
> >                        pc_i440fx_3_1_machine_options);
> >
> >  static void pc_i440fx_3_0_machine_options(MachineClass *m)
> >  {
> >      pc_i440fx_3_1_machine_options(m);
> > -    m->is_default = 0;
> > -    m->alias = NULL;
> >      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
> >  }
> >
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 4702bb13c472..e245db096dc1 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
> >      m->max_cpus = 288;
> >  }
> >
> > -static void pc_q35_3_1_machine_options(MachineClass *m)
> > +static void pc_q35_4_0_machine_options(MachineClass *m)
> >  {
> >      pc_q35_machine_options(m);
> >      m->alias = "q35";
> >  }
> >
> > +DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
> > +                   pc_q35_4_0_machine_options);
> > +
> > +static void pc_q35_3_1_machine_options(MachineClass *m)
> > +{
> > +    pc_q35_4_0_machine_options(m);
> > +    m->alias = NULL;
> > +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> > +}
> > +
> >  DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
> >                     pc_q35_3_1_machine_options);
> >
> >  static void pc_q35_3_0_machine_options(MachineClass *m)
> >  {
> >      pc_q35_3_1_machine_options(m);
> > -    m->alias = NULL;
> >      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
> >  }
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7afd1a175bf2..80d8498867a6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
> >      }                                                                \
> >      type_init(spapr_machine_register_##suffix)
> >
> > - /*
> > +/*
> > + * pseries-4.0
> > + */
> > +static void spapr_machine_4_0_instance_options(MachineState *machine)
> > +{
> > +}
> > +
> > +static void spapr_machine_4_0_class_options(MachineClass *mc)
> > +{
> > +    /* Defaults for the latest behaviour inherited from the base class */
> > +}
> > +
> > +DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> > +
> > +/*
> >   * pseries-3.1
> >   */
> > +#define SPAPR_COMPAT_3_1                                              \
> > +    HW_COMPAT_3_1
> > +
> >  static void spapr_machine_3_1_instance_options(MachineState *machine)
> >  {
> > +    spapr_machine_4_0_instance_options(machine);
> >  }
> >
> >  static void spapr_machine_3_1_class_options(MachineClass *mc)
> >  {
> > -    /* Defaults for the latest behaviour inherited from the base class */
> > +    spapr_machine_3_1_class_options(mc);
> > +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> >  }
> >
> > -DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
> > +DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> >
> >  /*
> >   * pseries-3.0
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index a0615a8b35f5..fd9d0b0542bb 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -651,6 +651,9 @@ bool css_migration_enabled(void)
> >      }                                                                         \
> >      type_init(ccw_machine_register_##suffix)
> >
> > +#define CCW_COMPAT_3_1 \
> > +        HW_COMPAT_3_1
> > +
> >  #define CCW_COMPAT_3_0 \
> >          HW_COMPAT_3_0
> >
> > @@ -742,14 +745,26 @@ bool css_migration_enabled(void)
> >              .value    = "0",\
> >          },
> >
> > +static void ccw_machine_4_0_instance_options(MachineState *machine)
> > +{
> > +}
> > +
> > +static void ccw_machine_4_0_class_options(MachineClass *mc)
> > +{
> > +}
> > +DEFINE_CCW_MACHINE(4_0, "4.0", true);
> > +
> >  static void ccw_machine_3_1_instance_options(MachineState *machine)
> >  {
> > +    ccw_machine_4_0_instance_options(machine);
> >  }
> >
> >  static void ccw_machine_3_1_class_options(MachineClass *mc)
> >  {
> > +    ccw_machine_4_0_class_options(mc);
> > +    SET_MACHINE_COMPAT(mc, CCW_COMPAT_3_1);
> >  }
> > -DEFINE_CCW_MACHINE(3_1, "3.1", true);
> > +DEFINE_CCW_MACHINE(3_1, "3.1", false);
> >
> >  static void ccw_machine_3_0_instance_options(MachineState *machine)
> >  {
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 6f4d5fc64704..70958328fe7a 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -1,6 +1,9 @@
> >  #ifndef HW_COMPAT_H
> >  #define HW_COMPAT_H
> >
> > +#define HW_COMPAT_3_1 \
> > +    /* empty */
> > +
> >  #define HW_COMPAT_3_0 \
> >      /* empty */
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 136fe497b6b2..cb645bf368a3 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >  int e820_get_num_entries(void);
> >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >
> > +#define PC_COMPAT_3_1 \
> > +    HW_COMPAT_3_1 \
> > +
> >  #define PC_COMPAT_3_0 \
> >      HW_COMPAT_3_0 \
> >      {\
> >
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [for-4.0 PATCH v3 8/9] q35/440fx/arm/spapr: Add QEMU 4.0 machine type

[Auger Eric]

Hi,

On 12/4/18 5:27 PM, Alex Williamson wrote:
> Including all machine types that might have a pcie-root-port.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
For the ARM virt machine
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


> ---
>  hw/arm/virt.c        |   19 +++++++++++++++++--
>  hw/i386/pc_piix.c    |   15 ++++++++++++---
>  hw/i386/pc_q35.c     |   13 +++++++++++--
>  hw/ppc/spapr.c       |   25 ++++++++++++++++++++++---
>  include/hw/compat.h  |    3 +++
>  include/hw/i386/pc.h |    3 +++
>  6 files changed, 68 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f69e7eb39977..beaf6bc43905 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1797,7 +1797,7 @@ static void machvirt_machine_init(void)
>  }
>  type_init(machvirt_machine_init);
>  
> -static void virt_3_1_instance_init(Object *obj)
> +static void virt_4_0_instance_init(Object *obj)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> @@ -1867,10 +1867,25 @@ static void virt_3_1_instance_init(Object *obj)
>      vms->irqmap = a15irqmap;
>  }
>  
> +static void virt_machine_4_0_options(MachineClass *mc)
> +{
> +}
> +DEFINE_VIRT_MACHINE_AS_LATEST(4, 0)
> +
> +#define VIRT_COMPAT_3_1 \
> +    HW_COMPAT_3_1
> +
> +static void virt_3_1_instance_init(Object *obj)
> +{
> +    virt_4_0_instance_init(obj);
> +}
> +
>  static void virt_machine_3_1_options(MachineClass *mc)
>  {
> +    virt_machine_4_0_options(mc);
> +    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_1);
>  }
> -DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
> +DEFINE_VIRT_MACHINE(3, 1)
>  
>  #define VIRT_COMPAT_3_0 \
>      HW_COMPAT_3_0
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7092d6d13f66..cfaa83ee2fad 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
>      machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
>  }
>  
> -static void pc_i440fx_3_1_machine_options(MachineClass *m)
> +static void pc_i440fx_4_0_machine_options(MachineClass *m)
>  {
>      pc_i440fx_machine_options(m);
>      m->alias = "pc";
>      m->is_default = 1;
>  }
>  
> +DEFINE_I440FX_MACHINE(v4_0, "pc-i440fx-4.0", NULL,
> +                      pc_i440fx_4_0_machine_options);
> +
> +static void pc_i440fx_3_1_machine_options(MachineClass *m)
> +{
> +    pc_i440fx_4_0_machine_options(m);
> +    m->is_default = 0;
> +    m->alias = NULL;
> +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> +}
> +
>  DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
>                        pc_i440fx_3_1_machine_options);
>  
>  static void pc_i440fx_3_0_machine_options(MachineClass *m)
>  {
>      pc_i440fx_3_1_machine_options(m);
> -    m->is_default = 0;
> -    m->alias = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 4702bb13c472..e245db096dc1 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
>      m->max_cpus = 288;
>  }
>  
> -static void pc_q35_3_1_machine_options(MachineClass *m)
> +static void pc_q35_4_0_machine_options(MachineClass *m)
>  {
>      pc_q35_machine_options(m);
>      m->alias = "q35";
>  }
>  
> +DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
> +                   pc_q35_4_0_machine_options);
> +
> +static void pc_q35_3_1_machine_options(MachineClass *m)
> +{
> +    pc_q35_4_0_machine_options(m);
> +    m->alias = NULL;
> +    SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> +}
> +
>  DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
>                     pc_q35_3_1_machine_options);
>  
>  static void pc_q35_3_0_machine_options(MachineClass *m)
>  {
>      pc_q35_3_1_machine_options(m);
> -    m->alias = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
>  }
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7afd1a175bf2..80d8498867a6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
>      }                                                                \
>      type_init(spapr_machine_register_##suffix)
>  
> - /*
> +/*
> + * pseries-4.0
> + */
> +static void spapr_machine_4_0_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_4_0_class_options(MachineClass *mc)
> +{
> +    /* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> +
> +/*
>   * pseries-3.1
>   */
> +#define SPAPR_COMPAT_3_1                                              \
> +    HW_COMPAT_3_1
> +
>  static void spapr_machine_3_1_instance_options(MachineState *machine)
>  {
> +    spapr_machine_4_0_instance_options(machine);
>  }
>  
>  static void spapr_machine_3_1_class_options(MachineClass *mc)
>  {
> -    /* Defaults for the latest behaviour inherited from the base class */
> +    spapr_machine_3_1_class_options(mc);
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
>  }
>  
> -DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
> +DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
>  
>  /*
>   * pseries-3.0
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 6f4d5fc64704..70958328fe7a 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -1,6 +1,9 @@
>  #ifndef HW_COMPAT_H
>  #define HW_COMPAT_H
>  
> +#define HW_COMPAT_3_1 \
> +    /* empty */
> +
>  #define HW_COMPAT_3_0 \
>      /* empty */
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 136fe497b6b2..cb645bf368a3 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> +#define PC_COMPAT_3_1 \
> +    HW_COMPAT_3_1 \
> +
>  #define PC_COMPAT_3_0 \
>      HW_COMPAT_3_0 \
>      {\
> 
> 

Re: [Qemu-devel] [for-4.0 PATCH v3 8/9] q35/440fx/arm/spapr: Add QEMU 4.0 machine type

[Eduardo Habkost]

On Tue, Dec 04, 2018 at 09:27:16AM -0700, Alex Williamson wrote:
> Including all machine types that might have a pcie-root-port.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
[...]
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7afd1a175bf2..80d8498867a6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
>      }                                                                \
>      type_init(spapr_machine_register_##suffix)
>  
> - /*
> +/*
> + * pseries-4.0
> + */
> +static void spapr_machine_4_0_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_4_0_class_options(MachineClass *mc)
> +{
> +    /* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> +
> +/*
>   * pseries-3.1
>   */
> +#define SPAPR_COMPAT_3_1                                              \
> +    HW_COMPAT_3_1
> +
>  static void spapr_machine_3_1_instance_options(MachineState *machine)
>  {
> +    spapr_machine_4_0_instance_options(machine);
>  }
>  
>  static void spapr_machine_3_1_class_options(MachineClass *mc)
>  {
> -    /* Defaults for the latest behaviour inherited from the base class */
> +    spapr_machine_3_1_class_options(mc);

Infinite recursion.  This is supposed to be calling
spapr_machine_4_0_class_options().  I will fix manually on
machine-next.


> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
>  }
[...]

-- 
Eduardo

Re: [Qemu-devel] [for-4.0 PATCH v3 8/9] q35/440fx/arm/spapr: Add QEMU 4.0 machine type

[Alex Williamson]

On Thu, 6 Dec 2018 17:12:40 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Dec 04, 2018 at 09:27:16AM -0700, Alex Williamson wrote:
> > Including all machine types that might have a pcie-root-port.
> > 
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Acked-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---  
> [...]
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7afd1a175bf2..80d8498867a6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3956,19 +3956,38 @@ static const TypeInfo spapr_machine_info = {
> >      }                                                                \
> >      type_init(spapr_machine_register_##suffix)
> >  
> > - /*
> > +/*
> > + * pseries-4.0
> > + */
> > +static void spapr_machine_4_0_instance_options(MachineState *machine)
> > +{
> > +}
> > +
> > +static void spapr_machine_4_0_class_options(MachineClass *mc)
> > +{
> > +    /* Defaults for the latest behaviour inherited from the base class */
> > +}
> > +
> > +DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> > +
> > +/*
> >   * pseries-3.1
> >   */
> > +#define SPAPR_COMPAT_3_1                                              \
> > +    HW_COMPAT_3_1
> > +
> >  static void spapr_machine_3_1_instance_options(MachineState *machine)
> >  {
> > +    spapr_machine_4_0_instance_options(machine);
> >  }
> >  
> >  static void spapr_machine_3_1_class_options(MachineClass *mc)
> >  {
> > -    /* Defaults for the latest behaviour inherited from the base class */
> > +    spapr_machine_3_1_class_options(mc);  
> 
> Infinite recursion.  This is supposed to be calling
> spapr_machine_4_0_class_options().  I will fix manually on
> machine-next.

Gack!  Thanks!

Alex

[Qemu-devel] [for-4.0 PATCH v3 9/9] pcie: Fast PCIe root ports for new machines

[Alex Williamson]

Change the default speed and width for new machine types to the
fastest and widest currently supported.  This should be compatible to
the PCIe 4.0 spec.  Pre-QEMU-4.0 machine types remain at 2.5GT/s, x1
width.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci-bridge/gen_pcie_root_port.c |    4 ++--
 include/hw/compat.h                |   10 +++++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index ca5418a89dd2..9766edb44596 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -125,9 +125,9 @@ static Property gen_rp_props[] = {
     DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort,
                      res_reserve.mem_pref_64, -1),
     DEFINE_PROP_PCIE_LINK_SPEED("x-speed", PCIESlot,
-                                speed, PCIE_LINK_SPEED_2_5),
+                                speed, PCIE_LINK_SPEED_16),
     DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
-                                width, PCIE_LINK_WIDTH_1),
+                                width, PCIE_LINK_WIDTH_32),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 70958328fe7a..702cc62277db 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,15 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_3_1 \
-    /* empty */
+    {\
+        .driver   = "pcie-root-port",\
+        .property = "speed",\
+        .value    = "2_5",\
+    },{\
+        .driver   = "pcie-root-port",\
+        .property = "width",\
+        .value    = "1",\
+    },
 
 #define HW_COMPAT_3_0 \
     /* empty */

Re: [Qemu-devel] [for-4.0 PATCH v3 9/9] pcie: Fast PCIe root ports for new machines

[Alex Williamson]

On Tue, 04 Dec 2018 09:27:28 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> Change the default speed and width for new machine types to the
> fastest and widest currently supported.  This should be compatible to
> the PCIe 4.0 spec.  Pre-QEMU-4.0 machine types remain at 2.5GT/s, x1
> width.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/pci-bridge/gen_pcie_root_port.c |    4 ++--
>  include/hw/compat.h                |   10 +++++++++-
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index ca5418a89dd2..9766edb44596 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -125,9 +125,9 @@ static Property gen_rp_props[] = {
>      DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort,
>                       res_reserve.mem_pref_64, -1),
>      DEFINE_PROP_PCIE_LINK_SPEED("x-speed", PCIESlot,
> -                                speed, PCIE_LINK_SPEED_2_5),
> +                                speed, PCIE_LINK_SPEED_16),
>      DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
> -                                width, PCIE_LINK_WIDTH_1),
> +                                width, PCIE_LINK_WIDTH_32),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 70958328fe7a..702cc62277db 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,15 @@
>  #define HW_COMPAT_H
>  
>  #define HW_COMPAT_3_1 \
> -    /* empty */
> +    {\
> +        .driver   = "pcie-root-port",\
> +        .property = "speed",\
> +        .value    = "2_5",\
> +    },{\
> +        .driver   = "pcie-root-port",\
> +        .property = "width",\
> +        .value    = "1",\
> +    },

Whoops, these should be x-speed and x-width too.  Will correct.  Thanks,

Alex

>  #define HW_COMPAT_3_0 \
>      /* empty */
> 

Re: [Qemu-devel] [for-4.0 PATCH v3 9/9] pcie: Fast PCIe root ports for new machines

[Auger Eric]

Hi

On 12/4/18 5:27 PM, Alex Williamson wrote:
> Change the default speed and width for new machine types to the
> fastest and widest currently supported.  This should be compatible to
> the PCIe 4.0 spec.  Pre-QEMU-4.0 machine types remain at 2.5GT/s, x1
> width.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  hw/pci-bridge/gen_pcie_root_port.c |    4 ++--
>  include/hw/compat.h                |   10 +++++++++-
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index ca5418a89dd2..9766edb44596 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -125,9 +125,9 @@ static Property gen_rp_props[] = {
>      DEFINE_PROP_SIZE("pref64-reserve", GenPCIERootPort,
>                       res_reserve.mem_pref_64, -1),
>      DEFINE_PROP_PCIE_LINK_SPEED("x-speed", PCIESlot,
> -                                speed, PCIE_LINK_SPEED_2_5),
> +                                speed, PCIE_LINK_SPEED_16),
>      DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
> -                                width, PCIE_LINK_WIDTH_1),
> +                                width, PCIE_LINK_WIDTH_32),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 70958328fe7a..702cc62277db 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,15 @@
>  #define HW_COMPAT_H
>  
>  #define HW_COMPAT_3_1 \
> -    /* empty */
> +    {\
> +        .driver   = "pcie-root-port",\
> +        .property = "speed",\
> +        .value    = "2_5",\
> +    },{\
> +        .driver   = "pcie-root-port",\
> +        .property = "width",\
> +        .value    = "1",\
> +    },
>  
>  #define HW_COMPAT_3_0 \
>      /* empty */
> 
>