From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: mjrosato@linux.ibm.com, linux-doc@vger.kernel.org,
airlied@linux.ie, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, kwankhede@nvidia.com,
vneethv@linux.ibm.com, agordeev@linux.ibm.com, hch@infradead.org,
kvm@vger.kernel.org, corbet@lwn.net, pasic@linux.ibm.com,
borntraeger@linux.ibm.com, intel-gfx@lists.freedesktop.org,
jjherne@linux.ibm.com, farman@linux.ibm.com,
jchrist@linux.ibm.com, gor@linux.ibm.com,
linux-s390@vger.kernel.org, hca@linux.ibm.com,
freude@linux.ibm.com, rodrigo.vivi@intel.com,
intel-gvt-dev@lists.freedesktop.org, akrowiak@linux.ibm.com,
cohuck@redhat.com, oberpar@linux.ibm.com, svens@linux.ibm.com
Subject: Re: [Intel-gfx] [RFT][PATCH v2 5/9] vfio/ap: Remove redundant pfn
Date: Wed, 6 Jul 2022 14:55:19 -0300 [thread overview]
Message-ID: <20220706175519.GM693670@nvidia.com> (raw)
In-Reply-To: <20220706062759.24946-6-nicolinc@nvidia.com>
On Tue, Jul 05, 2022 at 11:27:55PM -0700, Nicolin Chen wrote:
> The vfio_ap_ops code maintains both nib address and its PFN, which
> is redundant, merely because vfio_pin/unpin_pages API wanted pfn.
> Since vfio_pin/unpin_pages() now accept "iova", remove duplicated
> pfn code in their callers too.
I would describe this as renaming saved_pfn to saved_iova
> *
> * @vcpu: the object representing the vcpu executing the PQAP(AQIC) instruction.
> * @nib: the location for storing the nib address.
> - * @g_pfn: the location for storing the page frame number of the page containing
> - * the nib.
> *
> * When the PQAP(AQIC) instruction is executed, general register 2 contains the
> * address of the notification indicator byte (nib) used for IRQ notification.
> - * This function parses the nib from gr2 and calculates the page frame
> - * number for the guest of the page containing the nib. The values are
> - * stored in @nib and @g_pfn respectively.
> - *
> - * The g_pfn of the nib is then validated to ensure the nib address is valid.
> + * This function parses and validate the nib from gr2.
> *
> * Return: returns zero if the nib address is a valid; otherwise, returns
> * -EINVAL.
> */
> -static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, unsigned long *nib,
> - unsigned long *g_pfn)
> +static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, dma_addr_t *nib)
> {
> *nib = vcpu->run->s.regs.gprs[2];
> - *g_pfn = *nib >> PAGE_SHIFT;
>
> - if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *g_pfn)))
> + if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *nib >> PAGE_SHIFT)))
> return -EINVAL;
This existing code is fishy. nib is either an IOVA passed to
vfio_pin_pages() or a GFN passed to gfn_to_hva(). These are not the
same thing and are not interchangable - writing code like this assumes
that the guest is running with iommu=pt or no iommu.
Someone should look at it..
Otherwise it looks OK
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: kwankhede@nvidia.com, corbet@lwn.net, hca@linux.ibm.com,
gor@linux.ibm.com, agordeev@linux.ibm.com,
borntraeger@linux.ibm.com, svens@linux.ibm.com,
zhenyuw@linux.intel.com, zhi.a.wang@intel.com,
jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com,
airlied@linux.ie, daniel@ffwll.ch, farman@linux.ibm.com,
mjrosato@linux.ibm.com, pasic@linux.ibm.com,
vneethv@linux.ibm.com, oberpar@linux.ibm.com,
freude@linux.ibm.com, akrowiak@linux.ibm.com,
jjherne@linux.ibm.com, alex.williamson@redhat.com,
cohuck@redhat.com, kevin.tian@intel.com, hch@infradead.org,
jchrist@linux.ibm.com, kvm@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-s390@vger.kernel.org, intel-gvt-dev@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [RFT][PATCH v2 5/9] vfio/ap: Remove redundant pfn
Date: Wed, 6 Jul 2022 14:55:19 -0300 [thread overview]
Message-ID: <20220706175519.GM693670@nvidia.com> (raw)
In-Reply-To: <20220706062759.24946-6-nicolinc@nvidia.com>
On Tue, Jul 05, 2022 at 11:27:55PM -0700, Nicolin Chen wrote:
> The vfio_ap_ops code maintains both nib address and its PFN, which
> is redundant, merely because vfio_pin/unpin_pages API wanted pfn.
> Since vfio_pin/unpin_pages() now accept "iova", remove duplicated
> pfn code in their callers too.
I would describe this as renaming saved_pfn to saved_iova
> *
> * @vcpu: the object representing the vcpu executing the PQAP(AQIC) instruction.
> * @nib: the location for storing the nib address.
> - * @g_pfn: the location for storing the page frame number of the page containing
> - * the nib.
> *
> * When the PQAP(AQIC) instruction is executed, general register 2 contains the
> * address of the notification indicator byte (nib) used for IRQ notification.
> - * This function parses the nib from gr2 and calculates the page frame
> - * number for the guest of the page containing the nib. The values are
> - * stored in @nib and @g_pfn respectively.
> - *
> - * The g_pfn of the nib is then validated to ensure the nib address is valid.
> + * This function parses and validate the nib from gr2.
> *
> * Return: returns zero if the nib address is a valid; otherwise, returns
> * -EINVAL.
> */
> -static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, unsigned long *nib,
> - unsigned long *g_pfn)
> +static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, dma_addr_t *nib)
> {
> *nib = vcpu->run->s.regs.gprs[2];
> - *g_pfn = *nib >> PAGE_SHIFT;
>
> - if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *g_pfn)))
> + if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *nib >> PAGE_SHIFT)))
> return -EINVAL;
This existing code is fishy. nib is either an IOVA passed to
vfio_pin_pages() or a GFN passed to gfn_to_hva(). These are not the
same thing and are not interchangable - writing code like this assumes
that the guest is running with iommu=pt or no iommu.
Someone should look at it..
Otherwise it looks OK
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: mjrosato@linux.ibm.com, linux-doc@vger.kernel.org,
airlied@linux.ie, kevin.tian@intel.com,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
kwankhede@nvidia.com, vneethv@linux.ibm.com,
agordeev@linux.ibm.com, hch@infradead.org, kvm@vger.kernel.org,
corbet@lwn.net, pasic@linux.ibm.com, borntraeger@linux.ibm.com,
intel-gfx@lists.freedesktop.org, zhi.a.wang@intel.com,
jjherne@linux.ibm.com, farman@linux.ibm.com,
jchrist@linux.ibm.com, gor@linux.ibm.com,
linux-s390@vger.kernel.org, hca@linux.ibm.com,
alex.williamson@redhat.com, freude@linux.ibm.com,
rodrigo.vivi@intel.com, intel-gvt-dev@lists.freedesktop.org,
akrowiak@linux.ibm.com, tvrtko.ursulin@linux.intel.com,
cohuck@redhat.com, oberpar@linux.ibm.com, svens@linux.ibm.com
Subject: Re: [RFT][PATCH v2 5/9] vfio/ap: Remove redundant pfn
Date: Wed, 6 Jul 2022 14:55:19 -0300 [thread overview]
Message-ID: <20220706175519.GM693670@nvidia.com> (raw)
In-Reply-To: <20220706062759.24946-6-nicolinc@nvidia.com>
On Tue, Jul 05, 2022 at 11:27:55PM -0700, Nicolin Chen wrote:
> The vfio_ap_ops code maintains both nib address and its PFN, which
> is redundant, merely because vfio_pin/unpin_pages API wanted pfn.
> Since vfio_pin/unpin_pages() now accept "iova", remove duplicated
> pfn code in their callers too.
I would describe this as renaming saved_pfn to saved_iova
> *
> * @vcpu: the object representing the vcpu executing the PQAP(AQIC) instruction.
> * @nib: the location for storing the nib address.
> - * @g_pfn: the location for storing the page frame number of the page containing
> - * the nib.
> *
> * When the PQAP(AQIC) instruction is executed, general register 2 contains the
> * address of the notification indicator byte (nib) used for IRQ notification.
> - * This function parses the nib from gr2 and calculates the page frame
> - * number for the guest of the page containing the nib. The values are
> - * stored in @nib and @g_pfn respectively.
> - *
> - * The g_pfn of the nib is then validated to ensure the nib address is valid.
> + * This function parses and validate the nib from gr2.
> *
> * Return: returns zero if the nib address is a valid; otherwise, returns
> * -EINVAL.
> */
> -static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, unsigned long *nib,
> - unsigned long *g_pfn)
> +static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, dma_addr_t *nib)
> {
> *nib = vcpu->run->s.regs.gprs[2];
> - *g_pfn = *nib >> PAGE_SHIFT;
>
> - if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *g_pfn)))
> + if (kvm_is_error_hva(gfn_to_hva(vcpu->kvm, *nib >> PAGE_SHIFT)))
> return -EINVAL;
This existing code is fishy. nib is either an IOVA passed to
vfio_pin_pages() or a GFN passed to gfn_to_hva(). These are not the
same thing and are not interchangable - writing code like this assumes
that the guest is running with iommu=pt or no iommu.
Someone should look at it..
Otherwise it looks OK
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
next prev parent reply other threads:[~2022-07-06 17:55 UTC|newest]
Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 6:27 [RFT][PATCH v2 0/9] Update vfio_pin/unpin_pages API Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 6:27 ` [RFT][PATCH v2 1/9] vfio: Make vfio_unpin_pages() return void Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 6:54 ` [Intel-gfx] " Christoph Hellwig
2022-07-06 6:54 ` Christoph Hellwig
2022-07-06 15:52 ` Nicolin Chen
2022-07-06 15:52 ` Nicolin Chen
2022-07-06 16:45 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 16:45 ` Jason Gunthorpe
2022-07-06 16:45 ` Jason Gunthorpe
2022-07-06 17:38 ` [Intel-gfx] " Kirti Wankhede
2022-07-06 17:38 ` Kirti Wankhede
2022-07-06 17:38 ` Kirti Wankhede
2022-07-07 8:42 ` [Intel-gfx] " Tian, Kevin
2022-07-07 8:42 ` Tian, Kevin
2022-07-07 8:42 ` Tian, Kevin
2022-07-07 17:12 ` Nicolin Chen
2022-07-07 17:12 ` Nicolin Chen
2022-07-07 19:22 ` [Intel-gfx] " Jason Gunthorpe
2022-07-07 19:22 ` Jason Gunthorpe
2022-07-07 19:22 ` Jason Gunthorpe
2022-07-07 19:38 ` Nicolin Chen
2022-07-07 19:38 ` Nicolin Chen
2022-07-06 6:27 ` [RFT][PATCH v2 2/9] vfio/ap: Pass in physical address of ind to ap_aqic() Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 16:48 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 16:48 ` Jason Gunthorpe
2022-07-06 16:48 ` Jason Gunthorpe
2022-07-06 6:27 ` [RFT][PATCH v2 3/9] vfio/ccw: Only pass in contiguous pages Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 17:05 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 17:05 ` Jason Gunthorpe
2022-07-06 17:05 ` Jason Gunthorpe
2022-07-08 20:25 ` Eric Farman
2022-07-08 20:25 ` Eric Farman
2022-07-08 20:25 ` Eric Farman
2022-07-08 20:25 ` Eric Farman
2022-07-06 6:27 ` [RFT][PATCH v2 4/9] vfio: Pass in starting IOVA to vfio_pin/unpin_pages API Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 6:56 ` [Intel-gfx] " Christoph Hellwig
2022-07-06 6:56 ` Christoph Hellwig
2022-07-06 15:54 ` Nicolin Chen
2022-07-06 15:54 ` Nicolin Chen
2022-07-06 17:38 ` [Intel-gfx] " Kirti Wankhede
2022-07-06 17:38 ` Kirti Wankhede
2022-07-06 17:38 ` Kirti Wankhede
2022-07-06 17:49 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 17:49 ` Jason Gunthorpe
2022-07-06 17:49 ` Jason Gunthorpe
2022-07-06 17:58 ` Nicolin Chen
2022-07-06 17:58 ` Nicolin Chen
2022-07-06 19:21 ` Nicolin Chen
2022-07-06 19:21 ` Nicolin Chen
2022-07-07 8:46 ` [Intel-gfx] " Tian, Kevin
2022-07-07 8:46 ` Tian, Kevin
2022-07-07 17:04 ` Nicolin Chen
2022-07-07 17:04 ` Nicolin Chen
2022-07-08 20:27 ` Eric Farman
2022-07-08 20:27 ` Eric Farman
2022-07-06 6:27 ` [RFT][PATCH v2 5/9] vfio/ap: Remove redundant pfn Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 17:55 ` Jason Gunthorpe [this message]
2022-07-06 17:55 ` Jason Gunthorpe
2022-07-06 17:55 ` Jason Gunthorpe
2022-07-06 6:27 ` [RFT][PATCH v2 6/9] vfio/ccw: Change pa_pfn list to pa_iova list Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 17:59 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 17:59 ` Jason Gunthorpe
2022-07-06 17:59 ` Jason Gunthorpe
2022-07-08 20:26 ` Eric Farman
2022-07-08 20:26 ` Eric Farman
2022-07-06 6:27 ` [RFT][PATCH v2 7/9] vfio: Rename user_iova of vfio_dma_rw() Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 6:57 ` [Intel-gfx] " Christoph Hellwig
2022-07-06 6:57 ` Christoph Hellwig
2022-07-06 18:15 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 18:15 ` Jason Gunthorpe
2022-07-06 18:15 ` Jason Gunthorpe
2022-07-07 8:47 ` [Intel-gfx] " Tian, Kevin
2022-07-07 8:47 ` Tian, Kevin
2022-07-07 8:47 ` Tian, Kevin
2022-07-06 6:27 ` [RFT][PATCH v2 8/9] vfio/ccw: Add kmap_local_page() for memcpy Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 18:17 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 18:17 ` Jason Gunthorpe
2022-07-06 18:17 ` Jason Gunthorpe
2022-07-08 20:29 ` Eric Farman
2022-07-08 20:29 ` Eric Farman
2022-07-06 6:27 ` [RFT][PATCH v2 9/9] vfio: Replace phys_pfn with pages for vfio_pin_pages() Nicolin Chen
2022-07-06 6:27 ` Nicolin Chen
2022-07-06 6:57 ` [Intel-gfx] " Christoph Hellwig
2022-07-06 6:57 ` Christoph Hellwig
2022-07-06 17:39 ` [Intel-gfx] " Kirti Wankhede
2022-07-06 17:39 ` Kirti Wankhede
2022-07-06 17:39 ` Kirti Wankhede
2022-07-06 18:18 ` [Intel-gfx] " Jason Gunthorpe
2022-07-06 18:18 ` Jason Gunthorpe
2022-07-06 18:18 ` Jason Gunthorpe
2022-07-07 8:49 ` [Intel-gfx] " Tian, Kevin
2022-07-07 8:49 ` Tian, Kevin
2022-07-07 8:49 ` Tian, Kevin
2022-07-07 17:03 ` Nicolin Chen
2022-07-07 17:03 ` Nicolin Chen
2022-07-08 20:29 ` Eric Farman
2022-07-08 20:29 ` Eric Farman
2022-07-07 6:08 ` [Intel-gfx] [RFT][PATCH v2 0/9] Update vfio_pin/unpin_pages API Tian, Kevin
2022-07-07 6:08 ` Tian, Kevin
2022-07-07 6:08 ` Tian, Kevin
2022-07-07 6:17 ` Nicolin Chen
2022-07-07 6:17 ` Nicolin Chen
2022-07-08 7:24 ` [Intel-gfx] " Xu, Terrence
2022-07-08 7:24 ` Xu, Terrence
2022-07-08 7:24 ` Xu, Terrence
2022-07-08 20:20 ` Nicolin Chen
2022-07-08 20:20 ` Nicolin Chen
2022-07-08 20:30 ` Eric Farman
2022-07-08 20:30 ` Eric Farman
2022-07-08 20:31 ` Nicolin Chen
2022-07-08 20:31 ` Nicolin Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220706175519.GM693670@nvidia.com \
--to=jgg@nvidia.com \
--cc=agordeev@linux.ibm.com \
--cc=airlied@linux.ie \
--cc=akrowiak@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=cohuck@redhat.com \
--cc=corbet@lwn.net \
--cc=dri-devel@lists.freedesktop.org \
--cc=farman@linux.ibm.com \
--cc=freude@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=hch@infradead.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-gvt-dev@lists.freedesktop.org \
--cc=jchrist@linux.ibm.com \
--cc=jjherne@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mjrosato@linux.ibm.com \
--cc=nicolinc@nvidia.com \
--cc=oberpar@linux.ibm.com \
--cc=pasic@linux.ibm.com \
--cc=rodrigo.vivi@intel.com \
--cc=svens@linux.ibm.com \
--cc=vneethv@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.