| Message ID | 1542904555-1136-2-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/vfio: VFIO-AP interrupt control interception | expand |
On Thu, 22 Nov 2018 17:35:50 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > In the case we will enter QEMU through interception of instructions, > we will need to retrieve the AP devices. > The base device is the AP bridge. > > Let us implement a way to retrieve the AP Bridge from qtree. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > hw/s390x/ap-bridge.c | 12 ++++++++++++ > include/hw/s390x/ap-bridge.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c > index 3795d30..831ad5d 100644 > --- a/hw/s390x/ap-bridge.c > +++ b/hw/s390x/ap-bridge.c > @@ -14,6 +14,18 @@ > #include "hw/s390x/ap-bridge.h" > #include "cpu.h" > > +DeviceState *s390_get_ap_bridge(void) > +{ > + static DeviceState *apb; > + > + if (!apb) { > + apb = DEVICE(object_resolve_path(TYPE_AP_BRIDGE, NULL)); > + assert(apb != NULL); As you won't have an ap bridge if the ap feature is not provided, better do a quick exit if the feature bit is not set? I'd naively assume that this function can return NULL as well. > + } > + > + return apb; > +} > + > static char *ap_bus_get_dev_path(DeviceState *dev) > { > /* at most one */
On 11/22/18 11:35 AM, Pierre Morel wrote: > In the case we will enter QEMU through interception of instructions, > we will need to retrieve the AP devices. > The base device is the AP bridge. > > Let us implement a way to retrieve the AP Bridge from qtree. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > hw/s390x/ap-bridge.c | 12 ++++++++++++ > include/hw/s390x/ap-bridge.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c > index 3795d30..831ad5d 100644 > --- a/hw/s390x/ap-bridge.c > +++ b/hw/s390x/ap-bridge.c > @@ -14,6 +14,18 @@ > #include "hw/s390x/ap-bridge.h" > #include "cpu.h" > > +DeviceState *s390_get_ap_bridge(void) > +{ > + static DeviceState *apb; Since you are caching a reference to the bridge after retrieving it, why not make apb a global scope variable and initialize it in s390_init_ap() when the bridge is created. You can then declare it as an extern in the ap-bridge.h header file and you eliminate the need for this function. If you do make it a global var, I would name it ap_bridge; > + > + if (!apb) { > + apb = DEVICE(object_resolve_path(TYPE_AP_BRIDGE, NULL)); > + assert(apb != NULL); > + } > + > + return apb; > +} > + > static char *ap_bus_get_dev_path(DeviceState *dev) > { > /* at most one */ > diff --git a/include/hw/s390x/ap-bridge.h b/include/hw/s390x/ap-bridge.h > index 470e439..dacb877 100644 > --- a/include/hw/s390x/ap-bridge.h > +++ b/include/hw/s390x/ap-bridge.h > @@ -15,5 +15,6 @@ > #define TYPE_AP_BUS "ap-bus" > > void s390_init_ap(void); > +DeviceState *s390_get_ap_bridge(void); > > #endif >
On 29/11/2018 21:30, Tony Krowiak wrote: > On 11/22/18 11:35 AM, Pierre Morel wrote: >> In the case we will enter QEMU through interception of instructions, >> we will need to retrieve the AP devices. >> The base device is the AP bridge. >> >> Let us implement a way to retrieve the AP Bridge from qtree. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> hw/s390x/ap-bridge.c | 12 ++++++++++++ >> include/hw/s390x/ap-bridge.h | 1 + >> 2 files changed, 13 insertions(+) >> >> diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c >> index 3795d30..831ad5d 100644 >> --- a/hw/s390x/ap-bridge.c >> +++ b/hw/s390x/ap-bridge.c >> @@ -14,6 +14,18 @@ >> #include "hw/s390x/ap-bridge.h" >> #include "cpu.h" >> +DeviceState *s390_get_ap_bridge(void) >> +{ >> + static DeviceState *apb; > > Since you are caching a reference to the bridge after > retrieving it, why not make apb a global scope variable > and initialize it in s390_init_ap() when the bridge is > created. You can then declare it as an extern in the > ap-bridge.h header file and you eliminate the need for > this function. If you do make it a global var, I would > name it ap_bridge; We already had this discussion when implementing zPCI. I use a similar solution as it was decided at that time. Regards, Pierre
diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c index 3795d30..831ad5d 100644 --- a/hw/s390x/ap-bridge.c +++ b/hw/s390x/ap-bridge.c @@ -14,6 +14,18 @@ #include "hw/s390x/ap-bridge.h" #include "cpu.h" +DeviceState *s390_get_ap_bridge(void) +{ + static DeviceState *apb; + + if (!apb) { + apb = DEVICE(object_resolve_path(TYPE_AP_BRIDGE, NULL)); + assert(apb != NULL); + } + + return apb; +} + static char *ap_bus_get_dev_path(DeviceState *dev) { /* at most one */ diff --git a/include/hw/s390x/ap-bridge.h b/include/hw/s390x/ap-bridge.h index 470e439..dacb877 100644 --- a/include/hw/s390x/ap-bridge.h +++ b/include/hw/s390x/ap-bridge.h @@ -15,5 +15,6 @@ #define TYPE_AP_BUS "ap-bus" void s390_init_ap(void); +DeviceState *s390_get_ap_bridge(void); #endif
In the case we will enter QEMU through interception of instructions, we will need to retrieve the AP devices. The base device is the AP bridge. Let us implement a way to retrieve the AP Bridge from qtree. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- hw/s390x/ap-bridge.c | 12 ++++++++++++ include/hw/s390x/ap-bridge.h | 1 + 2 files changed, 13 insertions(+)