Re: [RFT][PATCH v1 1/6] vfio/ap: Pass in physical address of ind to ap_aqic()

[Harald Freudenberger]

On 2022-06-17 01:52, Nicolin Chen wrote:

The ap_aqic() is called by vfio_ap_irq_enable() where it passes in a
virt value that's casted from a physical address "h_nib". Inside the
ap_aqic(), it does virt_to_phys() again.

Since ap_aqic() needs a physical address, let's just pass in a pa of
ind directly. So change the "ind" to "pa_ind".

Signed-off-by: Nicolin Chen <nicol...@nvidia.com>
---
 arch/s390/include/asm/ap.h        | 6 +++---
 drivers/s390/crypto/ap_queue.c    | 2 +-
 drivers/s390/crypto/vfio_ap_ops.c | 7 ++++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
index b515cfa62bd9..f508f5025e38 100644
--- a/arch/s390/include/asm/ap.h
+++ b/arch/s390/include/asm/ap.h
@@ -227,13 +227,13 @@ struct ap_qirq_ctrl {
  * ap_aqic(): Control interruption for a specific AP.
  * @qid: The AP queue number
  * @qirqctrl: struct ap_qirq_ctrl (64 bit value)
- * @ind: The notification indicator byte
+ * @pa_ind: Physical address of the notification indicator byte
  *
  * Returns AP queue status.
  */
 static inline struct ap_queue_status ap_aqic(ap_qid_t qid,
                                             struct ap_qirq_ctrl qirqctrl,
-                                            void *ind)
+                                            phys_addr_t pa_ind)
 {
        unsigned long reg0 = qid | (3UL << 24);  /* fc 3UL is AQIC */
        union {

@@ -241,7 +241,7 @@ static inline struct ap_queue_status ap_aqic(ap_qid_t qid,

                struct ap_qirq_ctrl qirqctrl;
                struct ap_queue_status status;
        } reg1;
-       unsigned long reg2 = virt_to_phys(ind);
+       unsigned long reg2 = pa_ind;

        reg1.qirqctrl = qirqctrl;

diff --git a/drivers/s390/crypto/ap_queue.c b/drivers/s390/crypto/ap_queue.c

index c48b0db824e3..a32457b4cbb8 100644
--- a/drivers/s390/crypto/ap_queue.c
+++ b/drivers/s390/crypto/ap_queue.c

@@ -34,7 +34,7 @@ static int ap_queue_enable_irq(struct ap_queue *aq, void *ind)

        qirqctrl.ir = 1;
        qirqctrl.isc = AP_ISC;
-       status = ap_aqic(aq->qid, qirqctrl, ind);
+       status = ap_aqic(aq->qid, qirqctrl, virt_to_phys(ind));
        switch (status.response_code) {
        case AP_RESPONSE_NORMAL:
        case AP_RESPONSE_OTHERWISE_CHANGED:
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index a7d2a95796d3..bb869b28cebd 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -154,7 +154,7 @@ static struct ap_queue_status
vfio_ap_irq_disable(struct vfio_ap_queue *q)
        int retries = 5;

        do {
-               status = ap_aqic(q->apqn, aqic_gisa, NULL);
+               status = ap_aqic(q->apqn, aqic_gisa, 0);
                switch (status.response_code) {
                case AP_RESPONSE_OTHERWISE_CHANGED:
                case AP_RESPONSE_NORMAL:
@@ -245,7 +245,8 @@ static struct ap_queue_status
vfio_ap_irq_enable(struct vfio_ap_queue *q,
        struct kvm_s390_gisa *gisa;
        int nisc;
        struct kvm *kvm;
-       unsigned long h_nib, g_pfn, h_pfn;
+       unsigned long g_pfn, h_pfn;
+       phys_addr_t h_nib;
        int ret;

        /* Verify that the notification indicator byte address is valid */
@@ -290,7 +291,7 @@ static struct ap_queue_status
vfio_ap_irq_enable(struct vfio_ap_queue *q,
        aqic_gisa.ir = 1;
        aqic_gisa.gisa = (uint64_t)gisa >> 4;

-       status = ap_aqic(q->apqn, aqic_gisa, (void *)h_nib);
+       status = ap_aqic(q->apqn, aqic_gisa, h_nib);
        switch (status.response_code) {
        case AP_RESPONSE_NORMAL:
                /* See if we did clear older IRQ configuration */

Add my Reviewed-By: Harald Freudenberger <fre...@linux.ibm.com>