All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [for 4.1 PATCH 0/2] Disable PCIe ACS on older machines
@ 2019-07-30  9:37 Dr. David Alan Gilbert (git)
  2019-07-30 19:45   ` [Qemu-devel] [PULL " Michael S. Tsirkin
  2019-07-30 19:46   ` [Qemu-devel] [PULL " Michael S. Tsirkin
  0 siblings, 2 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-07-30  9:37 UTC (permalink / raw)
  To: qemu-devel, mst, ehabkost; +Cc: quintela

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled
@ 2019-07-30 19:45   ` Michael S. Tsirkin
  0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-07-30  9:37 UTC (permalink / raw)
  To: qemu-devel, mst, ehabkost; +Cc: quintela

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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines
@ 2019-07-30 19:46   ` Michael S. Tsirkin
  0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-07-30  9:37 UTC (permalink / raw)
  To: qemu-devel, mst, ehabkost; +Cc: quintela

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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled
  2019-07-30 19:45   ` [Qemu-devel] [PULL " Michael S. Tsirkin
  (?)
@ 2019-07-30 10:25   ` Igor Mammedov
  2019-07-30 11:13     ` Dr. David Alan Gilbert
  -1 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2019-07-30 10:25 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: quintela, qemu-devel, ehabkost, mst

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;
>  };
>  



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled
  2019-07-30 10:25   ` [Qemu-devel] [PATCH " Igor Mammedov
@ 2019-07-30 11:13     ` Dr. David Alan Gilbert
  2019-07-30 12:03       ` Igor Mammedov
  0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2019-07-30 11:13 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: peter.maydell, quintela, qemu-devel, ehabkost, mst

* 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled
  2019-07-30 11:13     ` Dr. David Alan Gilbert
@ 2019-07-30 12:03       ` Igor Mammedov
  2019-07-30 12:41         ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2019-07-30 12:03 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: peter.maydell, quintela, qemu-devel, ehabkost, mst

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines
  2019-07-30 19:46   ` [Qemu-devel] [PULL " Michael S. Tsirkin
  (?)
@ 2019-07-30 12:05   ` Igor Mammedov
  -1 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2019-07-30 12:05 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: quintela, qemu-devel, ehabkost, mst

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);
>  



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled
  2019-07-30 12:03       ` Igor Mammedov
@ 2019-07-30 12:41         ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2019-07-30 12:41 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: peter.maydell, quintela, qemu-devel, ehabkost, mst

* 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines
  2019-07-30 19:46   ` [Qemu-devel] [PULL " Michael S. Tsirkin
  (?)
  (?)
@ 2019-07-30 13:42   ` Juan Quintela
  -1 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2019-07-30 13:42 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, ehabkost, mst

"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>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled
  2019-07-30 19:45   ` [Qemu-devel] [PULL " Michael S. Tsirkin
  (?)
  (?)
@ 2019-07-30 13:42   ` Juan Quintela
  -1 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2019-07-30 13:42 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, ehabkost, mst

"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>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PULL 0/2] pci: bugfix
@ 2019-07-30 19:45 Michael S. Tsirkin
  2019-08-02 14:06 ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2019-07-30 19:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

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(-)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PULL 1/2] pcie_root_port: Allow ACS to be disabled
@ 2019-07-30 19:45   ` Michael S. Tsirkin
  0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2019-07-30 19:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Juan Quintela, Dr. David Alan Gilbert, Igor Mammedov

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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PULL 2/2] pcie_root_port: Disable ACS on older machines
@ 2019-07-30 19:46   ` Michael S. Tsirkin
  0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2019-07-30 19:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Juan Quintela,
	Dr. David Alan Gilbert, Igor Mammedov

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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PULL 0/2] pci: bugfix
  2019-07-30 19:45 [Qemu-devel] [PULL 0/2] pci: bugfix Michael S. Tsirkin
@ 2019-08-02 14:06 ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-08-02 14:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: QEMU Developers

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-08-02 14:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30 19:45 [Qemu-devel] [PULL 0/2] pci: bugfix Michael S. Tsirkin
2019-08-02 14:06 ` Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2019-07-30  9:37 [Qemu-devel] [for 4.1 PATCH 0/2] Disable PCIe ACS on older machines Dr. David Alan Gilbert (git)
2019-07-30  9:37 ` [Qemu-devel] [PATCH 1/2] pcie_root_port: Allow ACS to be disabled Dr. David Alan Gilbert (git)
2019-07-30 19:45   ` [Qemu-devel] [PULL " Michael S. Tsirkin
2019-07-30 10:25   ` [Qemu-devel] [PATCH " Igor Mammedov
2019-07-30 11:13     ` Dr. David Alan Gilbert
2019-07-30 12:03       ` Igor Mammedov
2019-07-30 12:41         ` Dr. David Alan Gilbert
2019-07-30 13:42   ` Juan Quintela
2019-07-30  9:37 ` [Qemu-devel] [PATCH 2/2] pcie_root_port: Disable ACS on older machines Dr. David Alan Gilbert (git)
2019-07-30 19:46   ` [Qemu-devel] [PULL " Michael S. Tsirkin
2019-07-30 12:05   ` [Qemu-devel] [PATCH " Igor Mammedov
2019-07-30 13:42   ` Juan Quintela

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.