* [PATCH v2] xio3130_upstream: Add ACS (Access Control Services) capability
@ 2022-08-16 9:16 Paul Schlacter
2022-08-16 10:10 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Paul Schlacter @ 2022-08-16 9:16 UTC (permalink / raw)
To: Michael S. Tsirkin, marcel.apfelbaum, fam, kwolf, stefanha,
k.jensen, f4bug, its, xypron.glpk, imammedo, qemu-devel, armbru,
kbusch, hreitz, ani
[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]
v1 -> v2:
- Allow ACS to be disabled.
- Suggested by Michael S. Tsirkin, use disable-acs to set property.
v1:
- Add ACS (Access Control Services) capability.
If it is a pcie device, check that all devices on the path from
the device to the root complex have ACS enabled, and then the
device will become an iommu_group.
it will have the effect of isolation
Signed-off-by: wangliang <wlfightup@gmail.com>
Signed-off-by: wangliang <wangliang40@baidu.com>
---
hw/pci-bridge/xio3130_upstream.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/pci-bridge/xio3130_upstream.c
b/hw/pci-bridge/xio3130_upstream.c
index 2df952222b..5433d06fb3 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -24,6 +24,7 @@
#include "hw/pci/msi.h"
#include "hw/pci/pcie.h"
#include "hw/pci/pcie_port.h"
+#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
@@ -59,6 +60,7 @@ static void xio3130_upstream_reset(DeviceState *qdev)
static void xio3130_upstream_realize(PCIDevice *d, Error **errp)
{
PCIEPort *p = PCIE_PORT(d);
+ PCIESlot *s = PCIE_SLOT(d);
int rc;
pci_bridge_initfn(d, TYPE_PCIE_BUS);
@@ -94,7 +96,9 @@ static void xio3130_upstream_realize(PCIDevice *d, Error
**errp)
goto err;
}
- pcie_acs_init(d, XIO3130_ACS_OFFSET);
+ if (!s->disable_acs) {
+ pcie_acs_init(d, XIO3130_ACS_OFFSET);
+ }
return;
err:
@@ -113,6 +117,11 @@ static void xio3130_upstream_exitfn(PCIDevice *d)
pci_bridge_exitfn(d);
}
+static Property xio3130_upstream_props[] = {
+ DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
+ DEFINE_PROP_END_OF_LIST()
+};
+
static const VMStateDescription vmstate_xio3130_upstream = {
.name = "xio3130-express-upstream-port",
.priority = MIG_PRI_PCI_BUS,
@@ -142,6 +151,7 @@ static void xio3130_upstream_class_init(ObjectClass
*klass, void *data)
dc->desc = "TI X3130 Upstream Port of PCI Express Switch";
dc->reset = xio3130_upstream_reset;
dc->vmsd = &vmstate_xio3130_upstream;
+ device_class_set_props(dc, xio3130_upstream_props);
}
static const TypeInfo xio3130_upstream_info = {
--
2.24.3 (Apple Git-128)
[-- Attachment #2: Type: text/html, Size: 23177 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xio3130_upstream: Add ACS (Access Control Services) capability
2022-08-16 9:16 [PATCH v2] xio3130_upstream: Add ACS (Access Control Services) capability Paul Schlacter
@ 2022-08-16 10:10 ` Michael S. Tsirkin
2022-08-16 12:38 ` Paul Schlacter
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2022-08-16 10:10 UTC (permalink / raw)
To: Paul Schlacter
Cc: marcel.apfelbaum, fam, kwolf, stefanha, k.jensen, f4bug, its,
xypron.glpk, imammedo, qemu-devel, armbru, kbusch, hreitz, ani
On Tue, Aug 16, 2022 at 05:16:38PM +0800, Paul Schlacter wrote:
> v1 -> v2:
> - Allow ACS to be disabled.
> - Suggested by Michael S. Tsirkin, use disable-acs to set property.
>
> v1:
> - Add ACS (Access Control Services) capability.
changelog generally after ---
>
> If it is a pcie device, check that all devices on the path from
Hmm I don't see any checks on a path. what does this refer to?
>
> the device to the root complex have ACS enabled, and then the
>
> device will become an iommu_group.
>
> it will have the effect of isolation
>
>
>
> Signed-off-by: wangliang <wlfightup@gmail.com>
>
> Signed-off-by: wangliang <wangliang40@baidu.com>
>
why two signatures?
>
> ---
>
> hw/pci-bridge/xio3130_upstream.c | 12 +++++++++++-
>
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
Patch has corrupted whitespace.
>
> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/
> xio3130_upstream.c
>
> index 2df952222b..5433d06fb3 100644
>
> --- a/hw/pci-bridge/xio3130_upstream.c
>
> +++ b/hw/pci-bridge/xio3130_upstream.c
>
> @@ -24,6 +24,7 @@
>
> #include "hw/pci/msi.h"
>
> #include "hw/pci/pcie.h"
>
> #include "hw/pci/pcie_port.h"
>
> +#include "hw/qdev-properties.h"
>
> #include "migration/vmstate.h"
>
> #include "qemu/module.h"
>
>
>
> @@ -59,6 +60,7 @@ static void xio3130_upstream_reset(DeviceState *qdev)
>
> static void xio3130_upstream_realize(PCIDevice *d, Error **errp)
>
> {
>
> PCIEPort *p = PCIE_PORT(d);
>
> + PCIESlot *s = PCIE_SLOT(d);
>
> int rc;
>
>
>
> pci_bridge_initfn(d, TYPE_PCIE_BUS);
>
> @@ -94,7 +96,9 @@ static void xio3130_upstream_realize(PCIDevice *d, Error
> **errp)
>
> goto err;
>
> }
>
>
>
> - pcie_acs_init(d, XIO3130_ACS_OFFSET);
>
> + if (!s->disable_acs) {
>
> + pcie_acs_init(d, XIO3130_ACS_OFFSET);
>
> + }
>
> return;
>
>
>
> err:
>
> @@ -113,6 +117,11 @@ static void xio3130_upstream_exitfn(PCIDevice *d)
>
> pci_bridge_exitfn(d);
>
> }
>
>
>
> +static Property xio3130_upstream_props[] = {
>
> + DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
>
> + DEFINE_PROP_END_OF_LIST()
>
> +};
>
> +
I'd say prefix the property with "x-".
>
> static const VMStateDescription vmstate_xio3130_upstream = {
>
> .name = "xio3130-express-upstream-port",
>
> .priority = MIG_PRI_PCI_BUS,
>
> @@ -142,6 +151,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass,
> void *data)
>
> dc->desc = "TI X3130 Upstream Port of PCI Express Switch";
>
> dc->reset = xio3130_upstream_reset;
>
> dc->vmsd = &vmstate_xio3130_upstream;
>
> + device_class_set_props(dc, xio3130_upstream_props);
>
> }
>
Seems to lack compat machinety for existing machine types.
>
>
> static const TypeInfo xio3130_upstream_info = {
>
> --
>
> 2.24.3 (Apple Git-128)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xio3130_upstream: Add ACS (Access Control Services) capability
2022-08-16 10:10 ` Michael S. Tsirkin
@ 2022-08-16 12:38 ` Paul Schlacter
0 siblings, 0 replies; 3+ messages in thread
From: Paul Schlacter @ 2022-08-16 12:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: marcel.apfelbaum, fam, kwolf, stefanha, k.jensen, f4bug, its,
xypron.glpk, imammedo, qemu-devel, armbru, kbusch, hreitz, ani
On Tue, Aug 16, 2022 at 6:11 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Aug 16, 2022 at 05:16:38PM +0800, Paul Schlacter wrote:
> > v1 -> v2:
> > - Allow ACS to be disabled.
> > - Suggested by Michael S. Tsirkin, use disable-acs to set property.
> >
> > v1:
> > - Add ACS (Access Control Services) capability.
>
> changelog generally after ---
>
> >
> > If it is a pcie device, check that all devices on the path from
>
>
> Hmm I don't see any checks on a path. what does this refer to?
pci_acs_path_enabled, this function in the Linux kernel,
it means that if the device is a PCIe device,
check the path from the device to the root complex. If ACS is all enabled,
the device will become an iommu_group.
acs determine whether it is a separate iommu_group.
>
> >
> > the device to the root complex have ACS enabled, and then the
> >
> > device will become an iommu_group.
> >
> > it will have the effect of isolation
> >
> >
> >
> > Signed-off-by: wangliang <wlfightup@gmail.com>
> >
> > Signed-off-by: wangliang <wangliang40@baidu.com>
> >
>
> why two signatures?
>
> >
> > ---
>
> >
> > hw/pci-bridge/xio3130_upstream.c | 12 +++++++++++-
> >
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
>
> Patch has corrupted whitespace.
>
> >
> > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/
> > xio3130_upstream.c
> >
> > index 2df952222b..5433d06fb3 100644
> >
> > --- a/hw/pci-bridge/xio3130_upstream.c
> >
> > +++ b/hw/pci-bridge/xio3130_upstream.c
> >
> > @@ -24,6 +24,7 @@
> >
> > #include "hw/pci/msi.h"
> >
> > #include "hw/pci/pcie.h"
> >
> > #include "hw/pci/pcie_port.h"
> >
> > +#include "hw/qdev-properties.h"
> >
> > #include "migration/vmstate.h"
> >
> > #include "qemu/module.h"
> >
> >
> >
> > @@ -59,6 +60,7 @@ static void xio3130_upstream_reset(DeviceState *qdev)
> >
> > static void xio3130_upstream_realize(PCIDevice *d, Error **errp)
> >
> > {
> >
> > PCIEPort *p = PCIE_PORT(d);
> >
> > + PCIESlot *s = PCIE_SLOT(d);
> >
> > int rc;
> >
> >
> >
> > pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >
> > @@ -94,7 +96,9 @@ static void xio3130_upstream_realize(PCIDevice *d, Error
> > **errp)
> >
> > goto err;
> >
> > }
> >
> >
> >
> > - pcie_acs_init(d, XIO3130_ACS_OFFSET);
> >
> > + if (!s->disable_acs) {
> >
> > + pcie_acs_init(d, XIO3130_ACS_OFFSET);
> >
> > + }
> >
> > return;
> >
> >
> >
> > err:
> >
> > @@ -113,6 +117,11 @@ static void xio3130_upstream_exitfn(PCIDevice *d)
> >
> > pci_bridge_exitfn(d);
> >
> > }
> >
> >
> >
> > +static Property xio3130_upstream_props[] = {
> >
> > + DEFINE_PROP_BOOL("disable-acs", PCIESlot, disable_acs, false),
> >
> > + DEFINE_PROP_END_OF_LIST()
> >
> > +};
> >
> > +
>
> I'd say prefix the property with "x-".
Do you want me to change it to this?
DEFINE_PROP_BOOL("x-disable-acs", PCIESlot, disable_acs, false),
disable_acs is a field in PCIESlot, which is also used by other code.
It may not be good to modify this field
>
>
> >
> > static const VMStateDescription vmstate_xio3130_upstream = {
> >
> > .name = "xio3130-express-upstream-port",
> >
> > .priority = MIG_PRI_PCI_BUS,
> >
> > @@ -142,6 +151,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass,
> > void *data)
> >
> > dc->desc = "TI X3130 Upstream Port of PCI Express Switch";
> >
> > dc->reset = xio3130_upstream_reset;
> >
> > dc->vmsd = &vmstate_xio3130_upstream;
> >
> > + device_class_set_props(dc, xio3130_upstream_props);
> >
> > }
> >
>
> Seems to lack compat machinety for existing machine types.
DEFINE_PROP_BOOL("x-disable-acs", PCIESlot, disable_acs, true),
If I change the default value to true, can I not add compat machine
>
>
> >
> >
> > static const TypeInfo xio3130_upstream_info = {
> >
> > --
> >
> > 2.24.3 (Apple Git-128)
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-16 12:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16 9:16 [PATCH v2] xio3130_upstream: Add ACS (Access Control Services) capability Paul Schlacter
2022-08-16 10:10 ` Michael S. Tsirkin
2022-08-16 12:38 ` Paul Schlacter
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.