| Message ID | 20220616235212.15185-2-nicolinc@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Update vfio_pin/unpin_pages API | expand |
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 <nicolinc@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 <freude@linux.ibm.com>
On Mon, Jun 20, 2022 at 12:00:53PM +0200, Harald Freudenberger wrote: > External email: Use caution opening links or attachments > > > 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 <nicolinc@nvidia.com> > Add my Reviewed-By: Harald Freudenberger <freude@linux.ibm.com> Will do. Thanks!
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 */
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 <nicolinc@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(-)