[Qemu-devel] [for 4.1 PATCH 0/2] Disable PCIe ACS on older machines

[Qemu-devel] [for 4.1 PATCH 0/2] Disable PCIe ACS on older machines

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

PCIe ACS (Access Control services) got added in 4.0 and broke
migration to and from 3.1 and earlier.  Fix it here
for older machine types, at the cost of breaking that compatibility
with 4.0.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


Dr. David Alan Gilbert (2):
  pcie_root_port: Allow ACS to be disabled
  pcie_root_port: Disable ACS on older machines

 hw/core/machine.c              | 1 +
 hw/pci-bridge/pcie_root_port.c | 3 ++-
 include/hw/pci/pcie_port.h     | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.21.0

[Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

ACS was added in 4.0 unconditionally, this breaks migration
compatibility.
Allow ACS to be disabled by adding a property that's
checked by pcie_root_port.

Unfortunately pcie-root-port doesn't have any instance data,
so there's no where for that flag to live, so stuff it into
PCIESlot.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/pci-bridge/pcie_root_port.c | 3 ++-
 include/hw/pci/pcie_port.h     | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 09019ca05d..1d8a778709 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -111,7 +111,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
     pcie_aer_root_init(d);
     rp_aer_vector_update(d);
 
-    if (rpc->acs_offset) {
+    if (rpc->acs_offset && !s->disable_acs) {
         pcie_acs_init(d, rpc->acs_offset);
     }
     return;
@@ -145,6 +145,7 @@ static void rp_exit(PCIDevice *d)
 static Property rp_props[] = {
     DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
                     QEMU_PCIE_SLTCAP_PCP_BITNR, true),
+    DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 09586f4641..7515430087 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -53,6 +53,8 @@ struct PCIESlot {
     PCIExpLinkSpeed speed;
     PCIExpLinkWidth width;
 
+    /* Disable ACS (really for a pcie_root_port) */
+    bool        disable_acs;
     QLIST_ENTRY(PCIESlot) next;
 };
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled

[Igor Mammedov]

On Tue, 30 Jul 2019 10:37:18 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> ACS was added in 4.0 unconditionally, this breaks migration
> compatibility.
> Allow ACS to be disabled by adding a property that's
> checked by pcie_root_port.
> 
> Unfortunately pcie-root-port doesn't have any instance data,
> so there's no where for that flag to live, so stuff it into
> PCIESlot.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/pci-bridge/pcie_root_port.c | 3 ++-
>  include/hw/pci/pcie_port.h     | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 09019ca05d..1d8a778709 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -111,7 +111,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
>      pcie_aer_root_init(d);
>      rp_aer_vector_update(d);
>  
> -    if (rpc->acs_offset) {
> +    if (rpc->acs_offset && !s->disable_acs) {

it's not like it would be used per instance,
so could we use class property and with rpc->disable_acs instead of PCIESlot?

>          pcie_acs_init(d, rpc->acs_offset);
>      }
>      return;
> @@ -145,6 +145,7 @@ static void rp_exit(PCIDevice *d)
>  static Property rp_props[] = {
>      DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
>                      QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> +    DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index 09586f4641..7515430087 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -53,6 +53,8 @@ struct PCIESlot {
>      PCIExpLinkSpeed speed;
>      PCIExpLinkWidth width;
>  
> +    /* Disable ACS (really for a pcie_root_port) */
> +    bool        disable_acs;
>      QLIST_ENTRY(PCIESlot) next;
>  };
>  

Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled

[Dr. David Alan Gilbert]

* Igor Mammedov (imammedo@redhat.com) wrote:
> On Tue, 30 Jul 2019 10:37:18 +0100
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > ACS was added in 4.0 unconditionally, this breaks migration
> > compatibility.
> > Allow ACS to be disabled by adding a property that's
> > checked by pcie_root_port.
> > 
> > Unfortunately pcie-root-port doesn't have any instance data,
> > so there's no where for that flag to live, so stuff it into
> > PCIESlot.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hw/pci-bridge/pcie_root_port.c | 3 ++-
> >  include/hw/pci/pcie_port.h     | 2 ++
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> > index 09019ca05d..1d8a778709 100644
> > --- a/hw/pci-bridge/pcie_root_port.c
> > +++ b/hw/pci-bridge/pcie_root_port.c
> > @@ -111,7 +111,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
> >      pcie_aer_root_init(d);
> >      rp_aer_vector_update(d);
> >  
> > -    if (rpc->acs_offset) {
> > +    if (rpc->acs_offset && !s->disable_acs) {
> 
> it's not like it would be used per instance,
> so could we use class property and with rpc->disable_acs instead of PCIESlot?

I'm not clear I understand how class properties help;
    object_class_property_add_bool
takes a getter/setter that takes an Object *  parameter;
my reading of that is that it's the instance data so has the same
problem.

Dave

> >          pcie_acs_init(d, rpc->acs_offset);
> >      }
> >      return;
> > @@ -145,6 +145,7 @@ static void rp_exit(PCIDevice *d)
> >  static Property rp_props[] = {
> >      DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
> >                      QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> > +    DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> > index 09586f4641..7515430087 100644
> > --- a/include/hw/pci/pcie_port.h
> > +++ b/include/hw/pci/pcie_port.h
> > @@ -53,6 +53,8 @@ struct PCIESlot {
> >      PCIExpLinkSpeed speed;
> >      PCIExpLinkWidth width;
> >  
> > +    /* Disable ACS (really for a pcie_root_port) */
> > +    bool        disable_acs;
> >      QLIST_ENTRY(PCIESlot) next;
> >  };
> >  
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled

[Igor Mammedov]

On Tue, 30 Jul 2019 12:13:06 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Igor Mammedov (imammedo@redhat.com) wrote:
> > On Tue, 30 Jul 2019 10:37:18 +0100
> > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > 
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > ACS was added in 4.0 unconditionally, this breaks migration
> > > compatibility.
> > > Allow ACS to be disabled by adding a property that's
> > > checked by pcie_root_port.
> > > 
> > > Unfortunately pcie-root-port doesn't have any instance data,
> > > so there's no where for that flag to live, so stuff it into
> > > PCIESlot.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > ---
> > >  hw/pci-bridge/pcie_root_port.c | 3 ++-
> > >  include/hw/pci/pcie_port.h     | 2 ++
> > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> > > index 09019ca05d..1d8a778709 100644
> > > --- a/hw/pci-bridge/pcie_root_port.c
> > > +++ b/hw/pci-bridge/pcie_root_port.c
> > > @@ -111,7 +111,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
> > >      pcie_aer_root_init(d);
> > >      rp_aer_vector_update(d);
> > >  
> > > -    if (rpc->acs_offset) {
> > > +    if (rpc->acs_offset && !s->disable_acs) {
> > 
> > it's not like it would be used per instance,
> > so could we use class property and with rpc->disable_acs instead of PCIESlot?
> 
> I'm not clear I understand how class properties help;
>     object_class_property_add_bool
> takes a getter/setter that takes an Object *  parameter;
> my reading of that is that it's the instance data so has the same
> problem.

it's possible to reach class data from setter/getter,
for example s390_cpu_model_class_register_props().

But it will be a bit more code than here and considering
that it's only field in parent type which is reachable through
root port specific property only it is also ok as is as a
fix to 4.1, so if you prefer to keep it like now

Reviewed-by: Igor Mammedov <imammedo@redhat.com>


> Dave
> 
> > >          pcie_acs_init(d, rpc->acs_offset);
> > >      }
> > >      return;
> > > @@ -145,6 +145,7 @@ static void rp_exit(PCIDevice *d)
> > >  static Property rp_props[] = {
> > >      DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
> > >                      QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> > > +    DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
> > >      DEFINE_PROP_END_OF_LIST()
> > >  };
> > >  
> > > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> > > index 09586f4641..7515430087 100644
> > > --- a/include/hw/pci/pcie_port.h
> > > +++ b/include/hw/pci/pcie_port.h
> > > @@ -53,6 +53,8 @@ struct PCIESlot {
> > >      PCIExpLinkSpeed speed;
> > >      PCIExpLinkWidth width;
> > >  
> > > +    /* Disable ACS (really for a pcie_root_port) */
> > > +    bool        disable_acs;
> > >      QLIST_ENTRY(PCIESlot) next;
> > >  };
> > >  
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled

[Dr. David Alan Gilbert]

* Igor Mammedov (imammedo@redhat.com) wrote:
> On Tue, 30 Jul 2019 12:13:06 +0100
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > * Igor Mammedov (imammedo@redhat.com) wrote:
> > > On Tue, 30 Jul 2019 10:37:18 +0100
> > > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > > 
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > ACS was added in 4.0 unconditionally, this breaks migration
> > > > compatibility.
> > > > Allow ACS to be disabled by adding a property that's
> > > > checked by pcie_root_port.
> > > > 
> > > > Unfortunately pcie-root-port doesn't have any instance data,
> > > > so there's no where for that flag to live, so stuff it into
> > > > PCIESlot.
> > > > 
> > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > > ---
> > > >  hw/pci-bridge/pcie_root_port.c | 3 ++-
> > > >  include/hw/pci/pcie_port.h     | 2 ++
> > > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> > > > index 09019ca05d..1d8a778709 100644
> > > > --- a/hw/pci-bridge/pcie_root_port.c
> > > > +++ b/hw/pci-bridge/pcie_root_port.c
> > > > @@ -111,7 +111,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
> > > >      pcie_aer_root_init(d);
> > > >      rp_aer_vector_update(d);
> > > >  
> > > > -    if (rpc->acs_offset) {
> > > > +    if (rpc->acs_offset && !s->disable_acs) {
> > > 
> > > it's not like it would be used per instance,
> > > so could we use class property and with rpc->disable_acs instead of PCIESlot?
> > 
> > I'm not clear I understand how class properties help;
> >     object_class_property_add_bool
> > takes a getter/setter that takes an Object *  parameter;
> > my reading of that is that it's the instance data so has the same
> > problem.
> 
> it's possible to reach class data from setter/getter,
> for example s390_cpu_model_class_register_props().

Ah OK.

> But it will be a bit more code than here and considering
> that it's only field in parent type which is reachable through
> root port specific property only it is also ok as is as a
> fix to 4.1, so if you prefer to keep it like now
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Thanks!

> 
> 
> > Dave
> > 
> > > >          pcie_acs_init(d, rpc->acs_offset);
> > > >      }
> > > >      return;
> > > > @@ -145,6 +145,7 @@ static void rp_exit(PCIDevice *d)
> > > >  static Property rp_props[] = {
> > > >      DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
> > > >                      QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> > > > +    DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
> > > >      DEFINE_PROP_END_OF_LIST()
> > > >  };
> > > >  
> > > > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> > > > index 09586f4641..7515430087 100644
> > > > --- a/include/hw/pci/pcie_port.h
> > > > +++ b/include/hw/pci/pcie_port.h
> > > > @@ -53,6 +53,8 @@ struct PCIESlot {
> > > >      PCIExpLinkSpeed speed;
> > > >      PCIExpLinkWidth width;
> > > >  
> > > > +    /* Disable ACS (really for a pcie_root_port) */
> > > > +    bool        disable_acs;
> > > >      QLIST_ENTRY(PCIESlot) next;
> > > >  };
> > > >  
> > > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> ACS was added in 4.0 unconditionally, this breaks migration
> compatibility.
> Allow ACS to be disabled by adding a property that's
> checked by pcie_root_port.
>
> Unfortunately pcie-root-port doesn't have any instance data,
> so there's no where for that flag to live, so stuff it into
> PCIESlot.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PULL 1/2] pcie_root_port: Allow ACS to be disabled

[Michael S. Tsirkin]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

ACS was added in 4.0 unconditionally, this breaks migration
compatibility.
Allow ACS to be disabled by adding a property that's
checked by pcie_root_port.

Unfortunately pcie-root-port doesn't have any instance data,
so there's no where for that flag to live, so stuff it into
PCIESlot.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20190730093719.12958-2-dgilbert@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-bridge/pcie_root_port.c | 3 ++-
 include/hw/pci/pcie_port.h     | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 09019ca05d..1d8a778709 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -111,7 +111,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
     pcie_aer_root_init(d);
     rp_aer_vector_update(d);
 
-    if (rpc->acs_offset) {
+    if (rpc->acs_offset && !s->disable_acs) {
         pcie_acs_init(d, rpc->acs_offset);
     }
     return;
@@ -145,6 +145,7 @@ static void rp_exit(PCIDevice *d)
 static Property rp_props[] = {
     DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
                     QEMU_PCIE_SLTCAP_PCP_BITNR, true),
+    DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 09586f4641..7515430087 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -53,6 +53,8 @@ struct PCIESlot {
     PCIExpLinkSpeed speed;
     PCIExpLinkWidth width;
 
+    /* Disable ACS (really for a pcie_root_port) */
+    bool        disable_acs;
     QLIST_ENTRY(PCIESlot) next;
 };
 
-- 
MST

[Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

ACS got added in 4.0 unconditionally,  that broke older<->4.0 migration
where there was a PCIe root port.
Fix this by turning it off for 3.1 and older machines; note this
fixes compatibility for older QEMUs but breaks compatibility with 4.0
for older machine types.

    machine type    source qemu   dest qemu
       3.1             3.1           4.0        broken
       3.1             3.1           4.1rc2     broken
       3.1             3.1           4.1+this   OK ++
       3.1             4.0           4.1rc2     OK
       3.1             4.0           4.1+this   broken --
       4.0             4.0           4.1rc2     OK
       4.0             4.0           4.1+this   OK

So we gain and lose; the consensus seems to be treat this as a
fix for older machine types.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/core/machine.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index c58a8e594e..26a5f30e6d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -52,6 +52,7 @@ GlobalProperty hw_compat_3_1[] = {
     { "virtio-blk-device", "discard", "false" },
     { "virtio-blk-device", "write-zeroes", "false" },
     { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
+    { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines

[Igor Mammedov]

On Tue, 30 Jul 2019 10:37:19 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> ACS got added in 4.0 unconditionally,  that broke older<->4.0 migration
> where there was a PCIe root port.
> Fix this by turning it off for 3.1 and older machines; note this
> fixes compatibility for older QEMUs but breaks compatibility with 4.0
> for older machine types.
> 
>     machine type    source qemu   dest qemu
>        3.1             3.1           4.0        broken
>        3.1             3.1           4.1rc2     broken
>        3.1             3.1           4.1+this   OK ++
>        3.1             4.0           4.1rc2     OK
>        3.1             4.0           4.1+this   broken --
>        4.0             4.0           4.1rc2     OK
>        4.0             4.0           4.1+this   OK
> 
> So we gain and lose; the consensus seems to be treat this as a
> fix for older machine types.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/core/machine.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index c58a8e594e..26a5f30e6d 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -52,6 +52,7 @@ GlobalProperty hw_compat_3_1[] = {
>      { "virtio-blk-device", "discard", "false" },
>      { "virtio-blk-device", "write-zeroes", "false" },
>      { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
> +    { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
>  };
>  const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>  

Re: [Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> ACS got added in 4.0 unconditionally,  that broke older<->4.0 migration
> where there was a PCIe root port.
> Fix this by turning it off for 3.1 and older machines; note this
> fixes compatibility for older QEMUs but breaks compatibility with 4.0
> for older machine types.
>
>     machine type    source qemu   dest qemu
>        3.1             3.1           4.0        broken
>        3.1             3.1           4.1rc2     broken
>        3.1             3.1           4.1+this   OK ++
>        3.1             4.0           4.1rc2     OK
>        3.1             4.0           4.1+this   broken --
>        4.0             4.0           4.1rc2     OK
>        4.0             4.0           4.1+this   OK
>
> So we gain and lose; the consensus seems to be treat this as a
> fix for older machine types.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PULL 2/2] pcie_root_port: Disable ACS on older machines

[Michael S. Tsirkin]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

ACS got added in 4.0 unconditionally,  that broke older<->4.0 migration
where there was a PCIe root port.
Fix this by turning it off for 3.1 and older machines; note this
fixes compatibility for older QEMUs but breaks compatibility with 4.0
for older machine types.

    machine type    source qemu   dest qemu
       3.1             3.1           4.0        broken
       3.1             3.1           4.1rc2     broken
       3.1             3.1           4.1+this   OK ++
       3.1             4.0           4.1rc2     OK
       3.1             4.0           4.1+this   broken --
       4.0             4.0           4.1rc2     OK
       4.0             4.0           4.1+this   OK

So we gain and lose; the consensus seems to be treat this as a
fix for older machine types.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20190730093719.12958-3-dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/core/machine.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index c4a2ab2282..28a475ad97 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -52,6 +52,7 @@ GlobalProperty hw_compat_3_1[] = {
     { "virtio-blk-device", "discard", "false" },
     { "virtio-blk-device", "write-zeroes", "false" },
     { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
+    { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
-- 
MST

[Qemu-devel] [PULL 0/2] pci: bugfix

[Michael S. Tsirkin]

The following changes since commit 22235bb609c18547cf6b215bad1f9d2ec56ad371:

  pc-dimm: fix crash when invalid slot number is used (2019-07-29 16:57:27 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to c8557f1b4873549fc231496f00003a9e5389c8dd:

  pcie_root_port: Disable ACS on older machines (2019-07-30 12:07:07 -0400)

----------------------------------------------------------------
pci: bugfix

A last minute fix to cross-version migration.
Better late than never.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Dr. David Alan Gilbert (2):
      pcie_root_port: Allow ACS to be disabled
      pcie_root_port: Disable ACS on older machines

 include/hw/pci/pcie_port.h     | 2 ++
 hw/core/machine.c              | 1 +
 hw/pci-bridge/pcie_root_port.c | 3 ++-
 3 files changed, 5 insertions(+), 1 deletion(-)

Re: [Qemu-devel] [PULL 0/2] pci: bugfix

[Peter Maydell]

On Tue, 30 Jul 2019 at 20:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit 22235bb609c18547cf6b215bad1f9d2ec56ad371:
>
>   pc-dimm: fix crash when invalid slot number is used (2019-07-29 16:57:27 -0400)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to c8557f1b4873549fc231496f00003a9e5389c8dd:
>
>   pcie_root_port: Disable ACS on older machines (2019-07-30 12:07:07 -0400)
>
> ----------------------------------------------------------------
> pci: bugfix
>
> A last minute fix to cross-version migration.
> Better late than never.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------
> Dr. David Alan Gilbert (2):
>       pcie_root_port: Allow ACS to be disabled
>       pcie_root_port: Disable ACS on older machines


Applied, thanks (before rc3 was tagged but I forgot to send this email).

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM