diff mbox series

[v2,6/6] s390x/vfio: ap: Implementing AP Queue Interrupt Control

Message ID 1542904555-1136-7-git-send-email-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x/vfio: VFIO-AP interrupt control interception | expand

Commit Message

Pierre Morel Nov. 22, 2018, 4:35 p.m. UTC
We intercept the PQAP(AQIC) instruction and transform
the guest's AQIC command parameters for the host AQIC
parameters.

Doing this we use the standard adapter interface to provide
the adapter NIB, indicator and ISC.

We define new structures, APCard and APQueue to keep track of
the ISC, route and indicator address and we add an array of
APCards in the APDevice.

We call the VFIO ioctl to set or clear the interruption
according to the "i" bit of the parameter.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 hw/vfio/ap.c                 | 117 ++++++++++++++++++++++++++++++++++++++++++-
 include/hw/s390x/ap-device.h |  63 +++++++++++++++++++++++
 target/s390x/kvm.c           |  31 ++++++++++++
 3 files changed, 210 insertions(+), 1 deletion(-)

Comments

Anthony Krowiak Nov. 29, 2018, 9:53 p.m. UTC | #1
On 11/22/18 11:35 AM, Pierre Morel wrote:
> We intercept the PQAP(AQIC) instruction and transform
> the guest's AQIC command parameters for the host AQIC
> parameters.
> 
> Doing this we use the standard adapter interface to provide
> the adapter NIB, indicator and ISC.
> 
> We define new structures, APCard and APQueue to keep track of
> the ISC, route and indicator address and we add an array of
> APCards in the APDevice.
> 
> We call the VFIO ioctl to set or clear the interruption
> according to the "i" bit of the parameter.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   hw/vfio/ap.c                 | 117 ++++++++++++++++++++++++++++++++++++++++++-
>   include/hw/s390x/ap-device.h |  63 +++++++++++++++++++++++
>   target/s390x/kvm.c           |  31 ++++++++++++
>   3 files changed, 210 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 94e5a1a..20d2e5f 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -4,6 +4,7 @@
>    * Copyright 2018 IBM Corp.
>    * Author(s): Tony Krowiak <akrowiak@linux.ibm.com>
>    *            Halil Pasic <pasic@linux.ibm.com>
> + *            Pierre Morel <pmorel@linux.ibm.com>
>    *
>    * This work is licensed under the terms of the GNU GPL, version 2 or (at
>    * your option) any later version. See the COPYING file in the top-level
> @@ -14,7 +15,6 @@
>   #include <sys/ioctl.h>
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
> -#include "hw/sysbus.h"
>   #include "hw/vfio/vfio.h"
>   #include "hw/vfio/vfio-common.h"
>   #include "hw/s390x/ap-device.h"
> @@ -27,6 +27,8 @@
>   #include "sysemu/sysemu.h"
>   #include "hw/s390x/ap-bridge.h"
>   #include "exec/address-spaces.h"
> +#include "hw/s390x/s390_flic.h"
> +#include "hw/s390x/css.h"
>   
>   #define VFIO_AP_DEVICE_TYPE      "vfio-ap"
>   
> @@ -35,6 +37,117 @@ typedef struct VFIOAPDevice {
>       VFIODevice vdev;
>   } VFIOAPDevice;
>   
> +static uint32_t ap_pqap_clear_irq(VFIODevice *vdev, APQueue *apq)
> +{
> +    struct vfio_ap_aqic param;
> +    uint32_t retval;
> +
> +    param.apqn = apq->apqn;
> +    param.isc = apq->isc;
> +    param.argsz = sizeof(param);
> +
> +    retval = ioctl(vdev->fd, VFIO_AP_CLEAR_IRQ, &param);
> +    switch (retval) {
> +    case 0:    /* Fall through and return the instruction status */

Unnecessary comment, we can see the code is going to fall through and
return status. Sorry, I know its a nit.

> +        release_indicator(&apq->routes.adapter, apq->nib);
> +    case -EIO: /* The case the PQAP instruction failed with status */

We know it's a case statement, so leave off the "The case the".

> +        return  param.status;
> +    default:   /* The case the ioctl call failed without isuing instruction */

Same here.

> +        break;
> +    }
> +    return ap_reg_set_status(AP_RC_INVALID_ADDR);
> +}
> +
> +static uint32_t ap_pqap_set_irq(VFIODevice *vdev, APQueue *apq, uint64_t g_nib)
> +{
> +    struct vfio_ap_aqic param;
> +    uint32_t retval;
> +    uint32_t id;
> +
> +    id = css_get_adapter_id(CSS_IO_ADAPTER_AP, apq->isc);
> +    if (id == -1) {
> +        return ap_reg_set_status(AP_RC_INVALID_ADDR);
> +    }
> +    apq->routes.adapter.adapter_id = id;
> +    apq->nib = get_indicator(ldq_p(&g_nib), 8);
> +
> +    retval = map_indicator(&apq->routes.adapter, apq->nib);
> +    if (retval) {
> +        return ap_reg_set_status(AP_RC_INVALID_ADDR);
> +    }
> +
> +    param.apqn = apq->apqn;
> +    param.isc = apq->isc;
> +    param.nib = g_nib;
> +    param.adapter_id = id;
> +    param.argsz = sizeof(param);
> +
> +    retval =  ioctl(vdev->fd, VFIO_AP_SET_IRQ, &param);
> +    switch (retval) {
> +    case -EIO: /* The case the PQAP instruction failed with status */

We know it's a case statement, so leave off the "The case the".

> +        release_indicator(&apq->routes.adapter, apq->nib);
> +    case 0:    /* Fall through and return the instruction status */

Unnecessary comment, the code speaks for itself.

> +        return  param.status;
> +    default:   /* The case the ioctl call failed without isuing instruction */

We know it's a case statement, so leave off the "The case the".

> +        break;
> +    }
> +    release_indicator(&apq->routes.adapter, apq->nib);
> +    return ap_reg_set_status(AP_RC_INVALID_ADDR);
> +}
> +
> +static int ap_pqap_aqic(CPUS390XState *env)
> +{
> +    uint64_t g_nib = env->regs[2];
> +    uint8_t apid = ap_reg_get_apid(env->regs[0]);
> +    uint8_t apqi = ap_reg_get_apqi(env->regs[0]);
> +    uint32_t retval;
> +    APDevice *ap = s390_get_ap();

To be consistent with the naming convention in the rest of
this file, can you name this variable 'apdev'?

> +    VFIODevice *vdev;
> +    VFIOAPDevice *ap_vdev;

To be consistent with the naming convention in the rest of
this file, can you name this variable 'vapdev'?

> +    APQueue *apq;
> +
> +    ap_vdev = DO_UPCAST(VFIOAPDevice, apdev, ap);

Where is 'apdev' defined/initialized?

> +    vdev = &ap_vdev->vdev;
> +    apq = &ap->card[apid].queue[apqi];
> +    apq->isc = ap_reg_get_isc(env->regs[1]);
> +    apq->apqn = (apid << 8) | apqi;
> +
> +    if (ap_reg_get_ir(env->regs[1])) {
> +        retval = ap_pqap_set_irq(vdev, apq, g_nib);
> +    } else {
> +        retval = ap_pqap_clear_irq(vdev, apq);
> +    }
> +
> +    env->regs[1] = retval;
> +    if (retval & AP_STATUS_RC_MASK) {
> +        return 3;
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * ap_pqap
> + * @env: environment pointing to registers
> + * return value: Code Condition
> + */
> +int ap_pqap(CPUS390XState *env)
> +{
> +    int cc = 0;
> +
> +    switch (ap_reg_get_fc(env->regs[0])) {
> +    case AQIC:
> +        if (!s390_has_feat(S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL)) {
> +            return -PGM_OPERATION;

Shouldn't this be PGM_SPECIFICATION?

> +        }
> +        cc = ap_pqap_aqic(env);
> +        break;
> +    default:
> +        return -PGM_OPERATION;
> +    }
> +    return cc;
> +}
> +
>   static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
>   {
>       vdev->needs_reset = false;
> @@ -106,6 +219,8 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
>           goto out_get_dev_err;
>       }
>   
> +    css_register_io_adapters(CSS_IO_ADAPTER_AP, true, false,
> +                             0, &error_abort);
>       return;
>   
>   out_get_dev_err:
> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
> index 5f3c840..8a36780 100644
> --- a/include/hw/s390x/ap-device.h
> +++ b/include/hw/s390x/ap-device.h
> @@ -12,8 +12,25 @@
>   
>   #define AP_DEVICE_TYPE       "ap-device"
>   
> +#define MAX_AP_CARD 256
> +#define MAX_AP_DOMAIN 256
> +
> +#include "hw/s390x/s390_flic.h"
> +#include "hw/s390x/css.h"
> +typedef struct APQueue {
> +    AdapterRoutes routes;
> +    IndAddr *nib;
> +    uint16_t apqn;
> +    uint8_t isc;
> +} APQueue;
> +
> +typedef struct APCard {
> +    APQueue queue[MAX_AP_DOMAIN];

This seems to be an unnecessary allocation of memory, particularly
if there is only a few queues. I understand this
maps to the concept of a matrix and makes for quick retrieval of
an APQueue, but I think it would be more efficient to use something
like a GTree, or a GHashTable both of which would save you space and
allow for fairly quick retrieval.

> +} APCard;
> +
>   typedef struct APDevice {
>       DeviceState parent_obj;
> +    APCard card[MAX_AP_CARD];

See comment above concerning the 'queue' field in the
APCard structure. I would also name this field 'cards'
since it represents all APCard objects.

>   } APDevice;
>   
>   #define AP_DEVICE(obj) \
> @@ -21,4 +38,50 @@ typedef struct APDevice {
>   
>   APDevice *s390_get_ap(void);
>   
> +#include "cpu.h"
> +int ap_pqap(CPUS390XState *env);
> +
> +/* AP PQAP commands definitions */
> +#define AQIC 0x03
> +
> +#define AP_AQIC_ZERO_BITS 0x0ff0000
> +
> +/* Register 0 hold the command and APQN */
> +static inline uint8_t ap_reg_get_apid(uint64_t r)
> +{
> +        return (r >> 8) & 0xff;
> +}
> +
> +static inline uint8_t ap_reg_get_apqi(uint64_t r)
> +{
> +        return r & 0xff;
> +}
> +
> +static inline uint16_t ap_reg_get_fc(uint64_t r)
> +{
> +        return (r >> 24) & 0xff;
> +}
> +
> +static inline uint16_t ap_reg_get_ir(uint64_t r)
> +{
> +        return (r >> 47) & 0x01;
> +}
> +
> +static inline uint16_t ap_reg_get_isc(uint64_t r)
> +{
> +        return r  & 0x7;
> +}
> +
> +/* AP status returned by the AP PQAP commands */
> +#define AP_STATUS_RC_MASK 0x00ff0000
> +#define AP_RC_APQN_INVALID 0x01
> +#define AP_RC_INVALID_ADDR 0x06
> +#define AP_RC_BAD_STATE    0x07
> +
> +/* Register 1 as input hold the AQIC information */
> +static inline uint32_t ap_reg_set_status(uint8_t status)

This function does not set anything, but returns an APQSW.
Maybe it should be named ap_reg_get_status_word or ap_reg_get_apqsw
or ap_reg_get_status.

> +{
> +        return status << 16;
> +}
> +
>   #endif /* HW_S390X_AP_DEVICE_H */
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 2ebf26a..a4b5d90 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -45,6 +45,7 @@
>   #include "trace.h"
>   #include "hw/s390x/s390-pci-inst.h"
>   #include "hw/s390x/s390-pci-bus.h"
> +#include "hw/s390x/ap-device.h"
>   #include "hw/s390x/ipl.h"
>   #include "hw/s390x/ebcdic.h"
>   #include "exec/memattrs.h"
> @@ -88,6 +89,7 @@
>   #define PRIV_B2_CHSC                    0x5f
>   #define PRIV_B2_SIGA                    0x74
>   #define PRIV_B2_XSCH                    0x76
> +#define PRIV_B2_PQAP                    0xaf
>   
>   #define PRIV_EB_SQBS                    0x8a
>   #define PRIV_EB_PCISTB                  0xd0
> @@ -1154,6 +1156,32 @@ static int kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run,
>       return 0;
>   }
>   
> +static int kvm_ap_pqap(S390CPU *cpu, uint16_t ipbh0)
> +{
> +    CPUS390XState *env = &cpu->env;
> +    int r;
> +
> +    if (env->psw.mask & PSW_MASK_PSTATE) {
> +        kvm_s390_program_interrupt(cpu, PGM_PRIVILEGED);
> +        return 0;
> +    }
> +
> +    if (env->regs[0] & AP_AQIC_ZERO_BITS) {
> +        kvm_s390_program_interrupt(cpu, PGM_SPECIFICATION);
> +        return 0;
> +    }

This check does not belong here; for example, if the PQAP(TAPQ)
instruction is intercepted and the T bit (bit 40) is set, a
specification exception would be erroneously recognized. This check
should be done in the ap_pqap_aqic() function.

> +
> +    r = ap_pqap(&cpu->env);
> +
> +    if (r < 0) {
> +        kvm_s390_program_interrupt(cpu, -r);
> +    } else {
> +        setcc(cpu, r);
> +    }
> +
> +    return 0;
> +}
> +
>   static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
>   {
>       CPUS390XState *env = &cpu->env;
> @@ -1216,6 +1244,9 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
>       case PRIV_B2_SCLP_CALL:
>           rc = kvm_sclp_service_call(cpu, run, ipbh0);
>           break;
> +    case PRIV_B2_PQAP:
> +        rc = kvm_ap_pqap(cpu, ipbh0);
> +        break;
>       default:
>           rc = -1;
>           DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1);
>
Cornelia Huck Nov. 30, 2018, 8:36 a.m. UTC | #2
On Thu, 29 Nov 2018 16:53:28 -0500
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> On 11/22/18 11:35 AM, Pierre Morel wrote:

> > +static uint32_t ap_pqap_clear_irq(VFIODevice *vdev, APQueue *apq)
> > +{
> > +    struct vfio_ap_aqic param;
> > +    uint32_t retval;
> > +
> > +    param.apqn = apq->apqn;
> > +    param.isc = apq->isc;
> > +    param.argsz = sizeof(param);
> > +
> > +    retval = ioctl(vdev->fd, VFIO_AP_CLEAR_IRQ, &param);
> > +    switch (retval) {
> > +    case 0:    /* Fall through and return the instruction status */  
> 
> Unnecessary comment, we can see the code is going to fall through and
> return status. Sorry, I know its a nit.

FWIW, some static code checking tools look for annotations like
/* Fallthrough */ as an indication that a fallthrough is actually
intended and not a coding error.
Pierre Morel Nov. 30, 2018, 11:54 a.m. UTC | #3
On 29/11/2018 22:53, Tony Krowiak wrote:
> On 11/22/18 11:35 AM, Pierre Morel wrote:
>> We intercept the PQAP(AQIC) instruction and transform

...

>> +static int ap_pqap_aqic(CPUS390XState *env)
>> +{
>> +    uint64_t g_nib = env->regs[2];
>> +    uint8_t apid = ap_reg_get_apid(env->regs[0]);
>> +    uint8_t apqi = ap_reg_get_apqi(env->regs[0]);
>> +    uint32_t retval;
>> +    APDevice *ap = s390_get_ap();
> 
> To be consistent with the naming convention in the rest of
> this file, can you name this variable 'apdev'?

OK

> 
>> +    VFIODevice *vdev;
>> +    VFIOAPDevice *ap_vdev;
> 
> To be consistent with the naming convention in the rest of
> this file, can you name this variable 'vapdev'?
> 
>> +    APQueue *apq;
>> +
>> +    ap_vdev = DO_UPCAST(VFIOAPDevice, apdev, ap);
> 
> Where is 'apdev' defined/initialized?

It is part of the VFIOAPDevice structure

> 
>> +    vdev = &ap_vdev->vdev;
>> +    apq = &ap->card[apid].queue[apqi];
>> +    apq->isc = ap_reg_get_isc(env->regs[1]);
>> +    apq->apqn = (apid << 8) | apqi;
>> +
>> +    if (ap_reg_get_ir(env->regs[1])) {
>> +        retval = ap_pqap_set_irq(vdev, apq, g_nib);
>> +    } else {
>> +        retval = ap_pqap_clear_irq(vdev, apq);
>> +    }
>> +
>> +    env->regs[1] = retval;
>> +    if (retval & AP_STATUS_RC_MASK) {
>> +        return 3;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * ap_pqap
>> + * @env: environment pointing to registers
>> + * return value: Code Condition
>> + */
>> +int ap_pqap(CPUS390XState *env)
>> +{
>> +    int cc = 0;
>> +
>> +    switch (ap_reg_get_fc(env->regs[0])) {
>> +    case AQIC:
>> +        if (!s390_has_feat(S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL)) {
>> +            return -PGM_OPERATION;
> 
> Shouldn't this be PGM_SPECIFICATION?

Before the patch, when PQAP is not intercepted we sent PGM_OPERATION.
I think we should continue doing the same as before if the feature is 
not activated.


> 
>> +        }

...snip...


>> +typedef struct APQueue {
>> +    AdapterRoutes routes;
>> +    IndAddr *nib;
>> +    uint16_t apqn;
>> +    uint8_t isc;
>> +} APQueue;
>> +
>> +typedef struct APCard {
>> +    APQueue queue[MAX_AP_DOMAIN];
> 
> This seems to be an unnecessary allocation of memory, particularly
> if there is only a few queues. I understand this
> maps to the concept of a matrix and makes for quick retrieval of
> an APQueue, but I think it would be more efficient to use something
> like a GTree, or a GHashTable both of which would save you space and
> allow for fairly quick retrieval.


OK.
We can optimize this later.

...

>> +/* Register 1 as input hold the AQIC information */
>> +static inline uint32_t ap_reg_set_status(uint8_t status)
> 
> This function does not set anything, but returns an APQSW.
> Maybe it should be named ap_reg_get_status_word or ap_reg_get_apqsw
> or ap_reg_get_status.

All these operations are "get" when retrieving informations from 
registers and "set" when setting information inside registers.


> 
...snip...
>>   }
>> +static int kvm_ap_pqap(S390CPU *cpu, uint16_t ipbh0)
>> +{
>> +    CPUS390XState *env = &cpu->env;
>> +    int r;
>> +
>> +    if (env->psw.mask & PSW_MASK_PSTATE) {
>> +        kvm_s390_program_interrupt(cpu, PGM_PRIVILEGED);
>> +        return 0;
>> +    }
>> +
>> +    if (env->regs[0] & AP_AQIC_ZERO_BITS) {
>> +        kvm_s390_program_interrupt(cpu, PGM_SPECIFICATION);
>> +        return 0;
>> +    }
> 
> This check does not belong here; for example, if the PQAP(TAPQ)
> instruction is intercepted and the T bit (bit 40) is set, a
> specification exception would be erroneously recognized. This check
> should be done in the ap_pqap_aqic() function.

Right, I will move the test.


Regards,
Pierre
diff mbox series

Patch

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 94e5a1a..20d2e5f 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -4,6 +4,7 @@ 
  * Copyright 2018 IBM Corp.
  * Author(s): Tony Krowiak <akrowiak@linux.ibm.com>
  *            Halil Pasic <pasic@linux.ibm.com>
+ *            Pierre Morel <pmorel@linux.ibm.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or (at
  * your option) any later version. See the COPYING file in the top-level
@@ -14,7 +15,6 @@ 
 #include <sys/ioctl.h>
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "hw/sysbus.h"
 #include "hw/vfio/vfio.h"
 #include "hw/vfio/vfio-common.h"
 #include "hw/s390x/ap-device.h"
@@ -27,6 +27,8 @@ 
 #include "sysemu/sysemu.h"
 #include "hw/s390x/ap-bridge.h"
 #include "exec/address-spaces.h"
+#include "hw/s390x/s390_flic.h"
+#include "hw/s390x/css.h"
 
 #define VFIO_AP_DEVICE_TYPE      "vfio-ap"
 
@@ -35,6 +37,117 @@  typedef struct VFIOAPDevice {
     VFIODevice vdev;
 } VFIOAPDevice;
 
+static uint32_t ap_pqap_clear_irq(VFIODevice *vdev, APQueue *apq)
+{
+    struct vfio_ap_aqic param;
+    uint32_t retval;
+
+    param.apqn = apq->apqn;
+    param.isc = apq->isc;
+    param.argsz = sizeof(param);
+
+    retval = ioctl(vdev->fd, VFIO_AP_CLEAR_IRQ, &param);
+    switch (retval) {
+    case 0:    /* Fall through and return the instruction status */
+        release_indicator(&apq->routes.adapter, apq->nib);
+    case -EIO: /* The case the PQAP instruction failed with status */
+        return  param.status;
+    default:   /* The case the ioctl call failed without isuing instruction */
+        break;
+    }
+    return ap_reg_set_status(AP_RC_INVALID_ADDR);
+}
+
+static uint32_t ap_pqap_set_irq(VFIODevice *vdev, APQueue *apq, uint64_t g_nib)
+{
+    struct vfio_ap_aqic param;
+    uint32_t retval;
+    uint32_t id;
+
+    id = css_get_adapter_id(CSS_IO_ADAPTER_AP, apq->isc);
+    if (id == -1) {
+        return ap_reg_set_status(AP_RC_INVALID_ADDR);
+    }
+    apq->routes.adapter.adapter_id = id;
+    apq->nib = get_indicator(ldq_p(&g_nib), 8);
+
+    retval = map_indicator(&apq->routes.adapter, apq->nib);
+    if (retval) {
+        return ap_reg_set_status(AP_RC_INVALID_ADDR);
+    }
+
+    param.apqn = apq->apqn;
+    param.isc = apq->isc;
+    param.nib = g_nib;
+    param.adapter_id = id;
+    param.argsz = sizeof(param);
+
+    retval =  ioctl(vdev->fd, VFIO_AP_SET_IRQ, &param);
+    switch (retval) {
+    case -EIO: /* The case the PQAP instruction failed with status */
+        release_indicator(&apq->routes.adapter, apq->nib);
+    case 0:    /* Fall through and return the instruction status */
+        return  param.status;
+    default:   /* The case the ioctl call failed without isuing instruction */
+        break;
+    }
+    release_indicator(&apq->routes.adapter, apq->nib);
+    return ap_reg_set_status(AP_RC_INVALID_ADDR);
+}
+
+static int ap_pqap_aqic(CPUS390XState *env)
+{
+    uint64_t g_nib = env->regs[2];
+    uint8_t apid = ap_reg_get_apid(env->regs[0]);
+    uint8_t apqi = ap_reg_get_apqi(env->regs[0]);
+    uint32_t retval;
+    APDevice *ap = s390_get_ap();
+    VFIODevice *vdev;
+    VFIOAPDevice *ap_vdev;
+    APQueue *apq;
+
+    ap_vdev = DO_UPCAST(VFIOAPDevice, apdev, ap);
+    vdev = &ap_vdev->vdev;
+    apq = &ap->card[apid].queue[apqi];
+    apq->isc = ap_reg_get_isc(env->regs[1]);
+    apq->apqn = (apid << 8) | apqi;
+
+    if (ap_reg_get_ir(env->regs[1])) {
+        retval = ap_pqap_set_irq(vdev, apq, g_nib);
+    } else {
+        retval = ap_pqap_clear_irq(vdev, apq);
+    }
+
+    env->regs[1] = retval;
+    if (retval & AP_STATUS_RC_MASK) {
+        return 3;
+    }
+
+    return 0;
+}
+
+/*
+ * ap_pqap
+ * @env: environment pointing to registers
+ * return value: Code Condition
+ */
+int ap_pqap(CPUS390XState *env)
+{
+    int cc = 0;
+
+    switch (ap_reg_get_fc(env->regs[0])) {
+    case AQIC:
+        if (!s390_has_feat(S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL)) {
+            return -PGM_OPERATION;
+        }
+        cc = ap_pqap_aqic(env);
+        break;
+    default:
+        return -PGM_OPERATION;
+    }
+    return cc;
+}
+
 static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
 {
     vdev->needs_reset = false;
@@ -106,6 +219,8 @@  static void vfio_ap_realize(DeviceState *dev, Error **errp)
         goto out_get_dev_err;
     }
 
+    css_register_io_adapters(CSS_IO_ADAPTER_AP, true, false,
+                             0, &error_abort);
     return;
 
 out_get_dev_err:
diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
index 5f3c840..8a36780 100644
--- a/include/hw/s390x/ap-device.h
+++ b/include/hw/s390x/ap-device.h
@@ -12,8 +12,25 @@ 
 
 #define AP_DEVICE_TYPE       "ap-device"
 
+#define MAX_AP_CARD 256
+#define MAX_AP_DOMAIN 256
+
+#include "hw/s390x/s390_flic.h"
+#include "hw/s390x/css.h"
+typedef struct APQueue {
+    AdapterRoutes routes;
+    IndAddr *nib;
+    uint16_t apqn;
+    uint8_t isc;
+} APQueue;
+
+typedef struct APCard {
+    APQueue queue[MAX_AP_DOMAIN];
+} APCard;
+
 typedef struct APDevice {
     DeviceState parent_obj;
+    APCard card[MAX_AP_CARD];
 } APDevice;
 
 #define AP_DEVICE(obj) \
@@ -21,4 +38,50 @@  typedef struct APDevice {
 
 APDevice *s390_get_ap(void);
 
+#include "cpu.h"
+int ap_pqap(CPUS390XState *env);
+
+/* AP PQAP commands definitions */
+#define AQIC 0x03
+
+#define AP_AQIC_ZERO_BITS 0x0ff0000
+
+/* Register 0 hold the command and APQN */
+static inline uint8_t ap_reg_get_apid(uint64_t r)
+{
+        return (r >> 8) & 0xff;
+}
+
+static inline uint8_t ap_reg_get_apqi(uint64_t r)
+{
+        return r & 0xff;
+}
+
+static inline uint16_t ap_reg_get_fc(uint64_t r)
+{
+        return (r >> 24) & 0xff;
+}
+
+static inline uint16_t ap_reg_get_ir(uint64_t r)
+{
+        return (r >> 47) & 0x01;
+}
+
+static inline uint16_t ap_reg_get_isc(uint64_t r)
+{
+        return r  & 0x7;
+}
+
+/* AP status returned by the AP PQAP commands */
+#define AP_STATUS_RC_MASK 0x00ff0000
+#define AP_RC_APQN_INVALID 0x01
+#define AP_RC_INVALID_ADDR 0x06
+#define AP_RC_BAD_STATE    0x07
+
+/* Register 1 as input hold the AQIC information */
+static inline uint32_t ap_reg_set_status(uint8_t status)
+{
+        return status << 16;
+}
+
 #endif /* HW_S390X_AP_DEVICE_H */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 2ebf26a..a4b5d90 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -45,6 +45,7 @@ 
 #include "trace.h"
 #include "hw/s390x/s390-pci-inst.h"
 #include "hw/s390x/s390-pci-bus.h"
+#include "hw/s390x/ap-device.h"
 #include "hw/s390x/ipl.h"
 #include "hw/s390x/ebcdic.h"
 #include "exec/memattrs.h"
@@ -88,6 +89,7 @@ 
 #define PRIV_B2_CHSC                    0x5f
 #define PRIV_B2_SIGA                    0x74
 #define PRIV_B2_XSCH                    0x76
+#define PRIV_B2_PQAP                    0xaf
 
 #define PRIV_EB_SQBS                    0x8a
 #define PRIV_EB_PCISTB                  0xd0
@@ -1154,6 +1156,32 @@  static int kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run,
     return 0;
 }
 
+static int kvm_ap_pqap(S390CPU *cpu, uint16_t ipbh0)
+{
+    CPUS390XState *env = &cpu->env;
+    int r;
+
+    if (env->psw.mask & PSW_MASK_PSTATE) {
+        kvm_s390_program_interrupt(cpu, PGM_PRIVILEGED);
+        return 0;
+    }
+
+    if (env->regs[0] & AP_AQIC_ZERO_BITS) {
+        kvm_s390_program_interrupt(cpu, PGM_SPECIFICATION);
+        return 0;
+    }
+
+    r = ap_pqap(&cpu->env);
+
+    if (r < 0) {
+        kvm_s390_program_interrupt(cpu, -r);
+    } else {
+        setcc(cpu, r);
+    }
+
+    return 0;
+}
+
 static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 {
     CPUS390XState *env = &cpu->env;
@@ -1216,6 +1244,9 @@  static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     case PRIV_B2_SCLP_CALL:
         rc = kvm_sclp_service_call(cpu, run, ipbh0);
         break;
+    case PRIV_B2_PQAP:
+        rc = kvm_ap_pqap(cpu, ipbh0);
+        break;
     default:
         rc = -1;
         DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1);