| Message ID | 20231129143746.6153-9-yishaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce a vfio driver over virtio devices | expand |
On Wed, 29 Nov 2023 16:37:45 +0200 Yishai Hadas <yishaih@nvidia.com> wrote: > Expose vfio_pci_iowrite/read##size() to let it be used by drivers. > > This functionality is needed to enable direct access to some physical > BAR of the device with the proper locks/checks in place. > > The next patches from this series will use this functionality on a data > path flow when a direct access to the BAR is needed. > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > --- > drivers/vfio/pci/vfio_pci_rdwr.c | 10 ++++++---- > include/linux/vfio_pci_core.h | 19 +++++++++++++++++++ > 2 files changed, 25 insertions(+), 4 deletions(-) I don't follow the inconsistency between this and the previous patch. Why did we move and rename the code to setup the barmap but we export the ioread/write functions in place? Thanks, Alex > diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c > index 6f08b3ecbb89..817ec9a89123 100644 > --- a/drivers/vfio/pci/vfio_pci_rdwr.c > +++ b/drivers/vfio/pci/vfio_pci_rdwr.c > @@ -38,7 +38,7 @@ > #define vfio_iowrite8 iowrite8 > > #define VFIO_IOWRITE(size) \ > -static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > bool test_mem, u##size val, void __iomem *io) \ > { \ > if (test_mem) { \ > @@ -55,7 +55,8 @@ static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > up_read(&vdev->memory_lock); \ > \ > return 0; \ > -} > +} \ > +EXPORT_SYMBOL_GPL(vfio_pci_iowrite##size); > > VFIO_IOWRITE(8) > VFIO_IOWRITE(16) > @@ -65,7 +66,7 @@ VFIO_IOWRITE(64) > #endif > > #define VFIO_IOREAD(size) \ > -static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > bool test_mem, u##size *val, void __iomem *io) \ > { \ > if (test_mem) { \ > @@ -82,7 +83,8 @@ static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > up_read(&vdev->memory_lock); \ > \ > return 0; \ > -} > +} \ > +EXPORT_SYMBOL_GPL(vfio_pci_ioread##size); > > VFIO_IOREAD(8) > VFIO_IOREAD(16) > diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h > index 67ac58e20e1d..22c915317788 100644 > --- a/include/linux/vfio_pci_core.h > +++ b/include/linux/vfio_pci_core.h > @@ -131,4 +131,23 @@ int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); > pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, > pci_channel_state_t state); > > +#define VFIO_IOWRITE_DECLATION(size) \ > +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > + bool test_mem, u##size val, void __iomem *io); > + > +VFIO_IOWRITE_DECLATION(8) > +VFIO_IOWRITE_DECLATION(16) > +VFIO_IOWRITE_DECLATION(32) > +#ifdef iowrite64 > +VFIO_IOWRITE_DECLATION(64) > +#endif > + > +#define VFIO_IOREAD_DECLATION(size) \ > +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > + bool test_mem, u##size *val, void __iomem *io); > + > +VFIO_IOREAD_DECLATION(8) > +VFIO_IOREAD_DECLATION(16) > +VFIO_IOREAD_DECLATION(32) > + > #endif /* VFIO_PCI_CORE_H */
On 30/11/2023 21:20, Alex Williamson wrote: > On Wed, 29 Nov 2023 16:37:45 +0200 > Yishai Hadas <yishaih@nvidia.com> wrote: > >> Expose vfio_pci_iowrite/read##size() to let it be used by drivers. >> >> This functionality is needed to enable direct access to some physical >> BAR of the device with the proper locks/checks in place. >> >> The next patches from this series will use this functionality on a data >> path flow when a direct access to the BAR is needed. >> >> Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >> --- >> drivers/vfio/pci/vfio_pci_rdwr.c | 10 ++++++---- >> include/linux/vfio_pci_core.h | 19 +++++++++++++++++++ >> 2 files changed, 25 insertions(+), 4 deletions(-) > > I don't follow the inconsistency between this and the previous patch. > Why did we move and rename the code to setup the barmap but we export > the ioread/write functions in place? Thanks, > > Alex The mount of code for barmap setup was quite small compared to the ioread/write functions. However, I agree, we can be consistent here and export in both cases the functions in place as part of vfio_pci_rdwr.c which is already part of vfio_pci_core.ko I may also rename in current patch vfio_pci_iowrite/read<xxx> to have the 'core' prefix as part of the functions names (i.e. vfio_pci_core_iowrite/read<xxx>) to be consistent with other exported core functions and adapt the callers to this name. Makes sense ? Yishai > > >> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c >> index 6f08b3ecbb89..817ec9a89123 100644 >> --- a/drivers/vfio/pci/vfio_pci_rdwr.c >> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c >> @@ -38,7 +38,7 @@ >> #define vfio_iowrite8 iowrite8 >> >> #define VFIO_IOWRITE(size) \ >> -static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ >> +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ >> bool test_mem, u##size val, void __iomem *io) \ >> { \ >> if (test_mem) { \ >> @@ -55,7 +55,8 @@ static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ >> up_read(&vdev->memory_lock); \ >> \ >> return 0; \ >> -} >> +} \ >> +EXPORT_SYMBOL_GPL(vfio_pci_iowrite##size); >> >> VFIO_IOWRITE(8) >> VFIO_IOWRITE(16) >> @@ -65,7 +66,7 @@ VFIO_IOWRITE(64) >> #endif >> >> #define VFIO_IOREAD(size) \ >> -static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ >> +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ >> bool test_mem, u##size *val, void __iomem *io) \ >> { \ >> if (test_mem) { \ >> @@ -82,7 +83,8 @@ static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ >> up_read(&vdev->memory_lock); \ >> \ >> return 0; \ >> -} >> +} \ >> +EXPORT_SYMBOL_GPL(vfio_pci_ioread##size); >> >> VFIO_IOREAD(8) >> VFIO_IOREAD(16) >> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h >> index 67ac58e20e1d..22c915317788 100644 >> --- a/include/linux/vfio_pci_core.h >> +++ b/include/linux/vfio_pci_core.h >> @@ -131,4 +131,23 @@ int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); >> pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, >> pci_channel_state_t state); >> >> +#define VFIO_IOWRITE_DECLATION(size) \ >> +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ >> + bool test_mem, u##size val, void __iomem *io); >> + >> +VFIO_IOWRITE_DECLATION(8) >> +VFIO_IOWRITE_DECLATION(16) >> +VFIO_IOWRITE_DECLATION(32) >> +#ifdef iowrite64 >> +VFIO_IOWRITE_DECLATION(64) >> +#endif >> + >> +#define VFIO_IOREAD_DECLATION(size) \ >> +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ >> + bool test_mem, u##size *val, void __iomem *io); >> + >> +VFIO_IOREAD_DECLATION(8) >> +VFIO_IOREAD_DECLATION(16) >> +VFIO_IOREAD_DECLATION(32) >> + >> #endif /* VFIO_PCI_CORE_H */ >
On Sun, 3 Dec 2023 16:14:15 +0200 Yishai Hadas <yishaih@nvidia.com> wrote: > On 30/11/2023 21:20, Alex Williamson wrote: > > On Wed, 29 Nov 2023 16:37:45 +0200 > > Yishai Hadas <yishaih@nvidia.com> wrote: > > > >> Expose vfio_pci_iowrite/read##size() to let it be used by drivers. > >> > >> This functionality is needed to enable direct access to some physical > >> BAR of the device with the proper locks/checks in place. > >> > >> The next patches from this series will use this functionality on a data > >> path flow when a direct access to the BAR is needed. > >> > >> Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > >> --- > >> drivers/vfio/pci/vfio_pci_rdwr.c | 10 ++++++---- > >> include/linux/vfio_pci_core.h | 19 +++++++++++++++++++ > >> 2 files changed, 25 insertions(+), 4 deletions(-) > > > > I don't follow the inconsistency between this and the previous patch. > > Why did we move and rename the code to setup the barmap but we export > > the ioread/write functions in place? Thanks, > > > > Alex > > The mount of code for barmap setup was quite small compared to the > ioread/write functions. > > However, I agree, we can be consistent here and export in both cases the > functions in place as part of vfio_pci_rdwr.c which is already part of > vfio_pci_core.ko > > I may also rename in current patch vfio_pci_iowrite/read<xxx> to have > the 'core' prefix as part of the functions names (i.e. > vfio_pci_core_iowrite/read<xxx>) to be consistent with other exported > core functions and adapt the callers to this name. > > Makes sense ? Yep. Thanks, Alex > >> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c > >> index 6f08b3ecbb89..817ec9a89123 100644 > >> --- a/drivers/vfio/pci/vfio_pci_rdwr.c > >> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c > >> @@ -38,7 +38,7 @@ > >> #define vfio_iowrite8 iowrite8 > >> > >> #define VFIO_IOWRITE(size) \ > >> -static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > >> +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > >> bool test_mem, u##size val, void __iomem *io) \ > >> { \ > >> if (test_mem) { \ > >> @@ -55,7 +55,8 @@ static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > >> up_read(&vdev->memory_lock); \ > >> \ > >> return 0; \ > >> -} > >> +} \ > >> +EXPORT_SYMBOL_GPL(vfio_pci_iowrite##size); > >> > >> VFIO_IOWRITE(8) > >> VFIO_IOWRITE(16) > >> @@ -65,7 +66,7 @@ VFIO_IOWRITE(64) > >> #endif > >> > >> #define VFIO_IOREAD(size) \ > >> -static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > >> +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > >> bool test_mem, u##size *val, void __iomem *io) \ > >> { \ > >> if (test_mem) { \ > >> @@ -82,7 +83,8 @@ static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > >> up_read(&vdev->memory_lock); \ > >> \ > >> return 0; \ > >> -} > >> +} \ > >> +EXPORT_SYMBOL_GPL(vfio_pci_ioread##size); > >> > >> VFIO_IOREAD(8) > >> VFIO_IOREAD(16) > >> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h > >> index 67ac58e20e1d..22c915317788 100644 > >> --- a/include/linux/vfio_pci_core.h > >> +++ b/include/linux/vfio_pci_core.h > >> @@ -131,4 +131,23 @@ int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); > >> pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, > >> pci_channel_state_t state); > >> > >> +#define VFIO_IOWRITE_DECLATION(size) \ > >> +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ > >> + bool test_mem, u##size val, void __iomem *io); > >> + > >> +VFIO_IOWRITE_DECLATION(8) > >> +VFIO_IOWRITE_DECLATION(16) > >> +VFIO_IOWRITE_DECLATION(32) > >> +#ifdef iowrite64 > >> +VFIO_IOWRITE_DECLATION(64) > >> +#endif > >> + > >> +#define VFIO_IOREAD_DECLATION(size) \ > >> +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ > >> + bool test_mem, u##size *val, void __iomem *io); > >> + > >> +VFIO_IOREAD_DECLATION(8) > >> +VFIO_IOREAD_DECLATION(16) > >> +VFIO_IOREAD_DECLATION(32) > >> + > >> #endif /* VFIO_PCI_CORE_H */ > > >
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c index 6f08b3ecbb89..817ec9a89123 100644 --- a/drivers/vfio/pci/vfio_pci_rdwr.c +++ b/drivers/vfio/pci/vfio_pci_rdwr.c @@ -38,7 +38,7 @@ #define vfio_iowrite8 iowrite8 #define VFIO_IOWRITE(size) \ -static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ bool test_mem, u##size val, void __iomem *io) \ { \ if (test_mem) { \ @@ -55,7 +55,8 @@ static int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ up_read(&vdev->memory_lock); \ \ return 0; \ -} +} \ +EXPORT_SYMBOL_GPL(vfio_pci_iowrite##size); VFIO_IOWRITE(8) VFIO_IOWRITE(16) @@ -65,7 +66,7 @@ VFIO_IOWRITE(64) #endif #define VFIO_IOREAD(size) \ -static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ bool test_mem, u##size *val, void __iomem *io) \ { \ if (test_mem) { \ @@ -82,7 +83,8 @@ static int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ up_read(&vdev->memory_lock); \ \ return 0; \ -} +} \ +EXPORT_SYMBOL_GPL(vfio_pci_ioread##size); VFIO_IOREAD(8) VFIO_IOREAD(16) diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 67ac58e20e1d..22c915317788 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -131,4 +131,23 @@ int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, pci_channel_state_t state); +#define VFIO_IOWRITE_DECLATION(size) \ +int vfio_pci_iowrite##size(struct vfio_pci_core_device *vdev, \ + bool test_mem, u##size val, void __iomem *io); + +VFIO_IOWRITE_DECLATION(8) +VFIO_IOWRITE_DECLATION(16) +VFIO_IOWRITE_DECLATION(32) +#ifdef iowrite64 +VFIO_IOWRITE_DECLATION(64) +#endif + +#define VFIO_IOREAD_DECLATION(size) \ +int vfio_pci_ioread##size(struct vfio_pci_core_device *vdev, \ + bool test_mem, u##size *val, void __iomem *io); + +VFIO_IOREAD_DECLATION(8) +VFIO_IOREAD_DECLATION(16) +VFIO_IOREAD_DECLATION(32) + #endif /* VFIO_PCI_CORE_H */
Expose vfio_pci_iowrite/read##size() to let it be used by drivers. This functionality is needed to enable direct access to some physical BAR of the device with the proper locks/checks in place. The next patches from this series will use this functionality on a data path flow when a direct access to the BAR is needed. Signed-off-by: Yishai Hadas <yishaih@nvidia.com> --- drivers/vfio/pci/vfio_pci_rdwr.c | 10 ++++++---- include/linux/vfio_pci_core.h | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-)