* [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.