| Message ID | 20231017134217.82497-7-yishaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce a vfio driver over virtio devices | expand |
Hi Yishai, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on v6.6-rc6 next-20231017] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yishai-Hadas/virtio-pci-Fix-common-config-map-for-modern-device/20231017-214450 base: linus/master patch link: https://lore.kernel.org/r/20231017134217.82497-7-yishaih%40nvidia.com patch subject: [PATCH V1 vfio 6/9] virtio-pci: Introduce APIs to execute legacy IO admin commands config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231018/202310180437.jo2csM6u-lkp@intel.com/config) compiler: alpha-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231018/202310180437.jo2csM6u-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310180437.jo2csM6u-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/virtio/virtio_pci_modern.c:731:5: warning: no previous prototype for 'virtio_pci_admin_list_query' [-Wmissing-prototypes] 731 | int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/virtio/virtio_pci_modern.c:758:5: warning: no previous prototype for 'virtio_pci_admin_list_use' [-Wmissing-prototypes] 758 | int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) | ^~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/virtio/virtio_pci_modern.c:786:5: warning: no previous prototype for 'virtio_pci_admin_legacy_io_write' [-Wmissing-prototypes] 786 | int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/virtio/virtio_pci_modern.c:831:5: warning: no previous prototype for 'virtio_pci_admin_legacy_io_read' [-Wmissing-prototypes] 831 | int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/virtio/virtio_pci_modern.c:877:5: warning: no previous prototype for 'virtio_pci_admin_legacy_io_notify_info' [-Wmissing-prototypes] 877 | int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/virtio_pci_admin_list_query +731 drivers/virtio/virtio_pci_modern.c 721 722 /* 723 * virtio_pci_admin_list_query - Provides to driver list of commands 724 * supported for the PCI VF. 725 * @dev: VF pci_dev 726 * @buf: buffer to hold the returned list 727 * @buf_size: size of the given buffer 728 * 729 * Returns 0 on success, or negative on failure. 730 */ > 731 int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) 732 { 733 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 734 struct virtio_admin_cmd cmd = {}; 735 struct scatterlist result_sg; 736 737 if (!virtio_dev) 738 return -ENODEV; 739 740 sg_init_one(&result_sg, buf, buf_size); 741 cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); 742 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 743 cmd.result_sg = &result_sg; 744 745 return vp_modern_admin_cmd_exec(virtio_dev, &cmd); 746 } 747 EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); 748 749 /* 750 * virtio_pci_admin_list_use - Provides to device list of commands 751 * used for the PCI VF. 752 * @dev: VF pci_dev 753 * @buf: buffer which holds the list 754 * @buf_size: size of the given buffer 755 * 756 * Returns 0 on success, or negative on failure. 757 */ > 758 int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) 759 { 760 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 761 struct virtio_admin_cmd cmd = {}; 762 struct scatterlist data_sg; 763 764 if (!virtio_dev) 765 return -ENODEV; 766 767 sg_init_one(&data_sg, buf, buf_size); 768 cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); 769 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 770 cmd.data_sg = &data_sg; 771 772 return vp_modern_admin_cmd_exec(virtio_dev, &cmd); 773 } 774 EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); 775 776 /* 777 * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device 778 * @dev: VF pci_dev 779 * @opcode: op code of the io write command 780 * @offset: starting byte offset within the registers to write to 781 * @size: size of the data to write 782 * @buf: buffer which holds the data 783 * 784 * Returns 0 on success, or negative on failure. 785 */ > 786 int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, 787 u8 offset, u8 size, u8 *buf) 788 { 789 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 790 struct virtio_admin_cmd_legacy_wr_data *data; 791 struct virtio_admin_cmd cmd = {}; 792 struct scatterlist data_sg; 793 int vf_id; 794 int ret; 795 796 if (!virtio_dev) 797 return -ENODEV; 798 799 vf_id = pci_iov_vf_id(pdev); 800 if (vf_id < 0) 801 return vf_id; 802 803 data = kzalloc(sizeof(*data) + size, GFP_KERNEL); 804 if (!data) 805 return -ENOMEM; 806 807 data->offset = offset; 808 memcpy(data->registers, buf, size); 809 sg_init_one(&data_sg, data, sizeof(*data) + size); 810 cmd.opcode = cpu_to_le16(opcode); 811 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 812 cmd.group_member_id = cpu_to_le64(vf_id + 1); 813 cmd.data_sg = &data_sg; 814 ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); 815 816 kfree(data); 817 return ret; 818 } 819 EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); 820 821 /* 822 * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device 823 * @dev: VF pci_dev 824 * @opcode: op code of the io read command 825 * @offset: starting byte offset within the registers to read from 826 * @size: size of the data to be read 827 * @buf: buffer to hold the returned data 828 * 829 * Returns 0 on success, or negative on failure. 830 */ > 831 int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, 832 u8 offset, u8 size, u8 *buf) 833 { 834 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 835 struct virtio_admin_cmd_legacy_rd_data *data; 836 struct scatterlist data_sg, result_sg; 837 struct virtio_admin_cmd cmd = {}; 838 int vf_id; 839 int ret; 840 841 if (!virtio_dev) 842 return -ENODEV; 843 844 vf_id = pci_iov_vf_id(pdev); 845 if (vf_id < 0) 846 return vf_id; 847 848 data = kzalloc(sizeof(*data), GFP_KERNEL); 849 if (!data) 850 return -ENOMEM; 851 852 data->offset = offset; 853 sg_init_one(&data_sg, data, sizeof(*data)); 854 sg_init_one(&result_sg, buf, size); 855 cmd.opcode = cpu_to_le16(opcode); 856 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 857 cmd.group_member_id = cpu_to_le64(vf_id + 1); 858 cmd.data_sg = &data_sg; 859 cmd.result_sg = &result_sg; 860 ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); 861 862 kfree(data); 863 return ret; 864 } 865 EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); 866 867 /* 868 * virtio_pci_admin_legacy_io_notify_info - Read the queue notification 869 * information for legacy interface 870 * @dev: VF pci_dev 871 * @req_bar_flags: requested bar flags 872 * @bar: on output the BAR number of the member device 873 * @bar_offset: on output the offset within bar 874 * 875 * Returns 0 on success, or negative on failure. 876 */ > 877 int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, 878 u8 req_bar_flags, u8 *bar, 879 u64 *bar_offset) 880 { 881 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 882 struct virtio_admin_cmd_notify_info_result *result; 883 struct virtio_admin_cmd cmd = {}; 884 struct scatterlist result_sg; 885 int vf_id; 886 int ret; 887 888 if (!virtio_dev) 889 return -ENODEV; 890 891 vf_id = pci_iov_vf_id(pdev); 892 if (vf_id < 0) 893 return vf_id; 894 895 result = kzalloc(sizeof(*result), GFP_KERNEL); 896 if (!result) 897 return -ENOMEM; 898 899 sg_init_one(&result_sg, result, sizeof(*result)); 900 cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); 901 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 902 cmd.group_member_id = cpu_to_le64(vf_id + 1); 903 cmd.result_sg = &result_sg; 904 ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); 905 if (!ret) { 906 struct virtio_admin_cmd_notify_info_data *entry; 907 int i; 908 909 ret = -ENOENT; 910 for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { 911 entry = &result->entries[i]; 912 if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) 913 break; 914 if (entry->flags != req_bar_flags) 915 continue; 916 *bar = entry->bar; 917 *bar_offset = le64_to_cpu(entry->offset); 918 ret = 0; 919 break; 920 } 921 } 922 923 kfree(result); 924 return ret; 925 } 926 EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); 927
Hi Yishai, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on v6.6-rc6] [cannot apply to next-20231020] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yishai-Hadas/virtio-pci-Fix-common-config-map-for-modern-device/20231017-214450 base: linus/master patch link: https://lore.kernel.org/r/20231017134217.82497-7-yishaih%40nvidia.com patch subject: [PATCH V1 vfio 6/9] virtio-pci: Introduce APIs to execute legacy IO admin commands config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231022/202310220842.ADAIiZsO-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231022/202310220842.ADAIiZsO-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310220842.ADAIiZsO-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/virtio/virtio_pci_modern.c:731:5: warning: no previous prototype for function 'virtio_pci_admin_list_query' [-Wmissing-prototypes] int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) ^ drivers/virtio/virtio_pci_modern.c:731:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) ^ static >> drivers/virtio/virtio_pci_modern.c:758:5: warning: no previous prototype for function 'virtio_pci_admin_list_use' [-Wmissing-prototypes] int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) ^ drivers/virtio/virtio_pci_modern.c:758:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) ^ static >> drivers/virtio/virtio_pci_modern.c:786:5: warning: no previous prototype for function 'virtio_pci_admin_legacy_io_write' [-Wmissing-prototypes] int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, ^ drivers/virtio/virtio_pci_modern.c:786:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, ^ static >> drivers/virtio/virtio_pci_modern.c:831:5: warning: no previous prototype for function 'virtio_pci_admin_legacy_io_read' [-Wmissing-prototypes] int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, ^ drivers/virtio/virtio_pci_modern.c:831:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, ^ static >> drivers/virtio/virtio_pci_modern.c:877:5: warning: no previous prototype for function 'virtio_pci_admin_legacy_io_notify_info' [-Wmissing-prototypes] int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, ^ drivers/virtio/virtio_pci_modern.c:877:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, ^ static 5 warnings generated. vim +/virtio_pci_admin_list_query +731 drivers/virtio/virtio_pci_modern.c 721 722 /* 723 * virtio_pci_admin_list_query - Provides to driver list of commands 724 * supported for the PCI VF. 725 * @dev: VF pci_dev 726 * @buf: buffer to hold the returned list 727 * @buf_size: size of the given buffer 728 * 729 * Returns 0 on success, or negative on failure. 730 */ > 731 int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) 732 { 733 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 734 struct virtio_admin_cmd cmd = {}; 735 struct scatterlist result_sg; 736 737 if (!virtio_dev) 738 return -ENODEV; 739 740 sg_init_one(&result_sg, buf, buf_size); 741 cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); 742 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 743 cmd.result_sg = &result_sg; 744 745 return vp_modern_admin_cmd_exec(virtio_dev, &cmd); 746 } 747 EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); 748 749 /* 750 * virtio_pci_admin_list_use - Provides to device list of commands 751 * used for the PCI VF. 752 * @dev: VF pci_dev 753 * @buf: buffer which holds the list 754 * @buf_size: size of the given buffer 755 * 756 * Returns 0 on success, or negative on failure. 757 */ > 758 int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) 759 { 760 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 761 struct virtio_admin_cmd cmd = {}; 762 struct scatterlist data_sg; 763 764 if (!virtio_dev) 765 return -ENODEV; 766 767 sg_init_one(&data_sg, buf, buf_size); 768 cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); 769 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 770 cmd.data_sg = &data_sg; 771 772 return vp_modern_admin_cmd_exec(virtio_dev, &cmd); 773 } 774 EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); 775 776 /* 777 * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device 778 * @dev: VF pci_dev 779 * @opcode: op code of the io write command 780 * @offset: starting byte offset within the registers to write to 781 * @size: size of the data to write 782 * @buf: buffer which holds the data 783 * 784 * Returns 0 on success, or negative on failure. 785 */ > 786 int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, 787 u8 offset, u8 size, u8 *buf) 788 { 789 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 790 struct virtio_admin_cmd_legacy_wr_data *data; 791 struct virtio_admin_cmd cmd = {}; 792 struct scatterlist data_sg; 793 int vf_id; 794 int ret; 795 796 if (!virtio_dev) 797 return -ENODEV; 798 799 vf_id = pci_iov_vf_id(pdev); 800 if (vf_id < 0) 801 return vf_id; 802 803 data = kzalloc(sizeof(*data) + size, GFP_KERNEL); 804 if (!data) 805 return -ENOMEM; 806 807 data->offset = offset; 808 memcpy(data->registers, buf, size); 809 sg_init_one(&data_sg, data, sizeof(*data) + size); 810 cmd.opcode = cpu_to_le16(opcode); 811 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 812 cmd.group_member_id = cpu_to_le64(vf_id + 1); 813 cmd.data_sg = &data_sg; 814 ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); 815 816 kfree(data); 817 return ret; 818 } 819 EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); 820 821 /* 822 * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device 823 * @dev: VF pci_dev 824 * @opcode: op code of the io read command 825 * @offset: starting byte offset within the registers to read from 826 * @size: size of the data to be read 827 * @buf: buffer to hold the returned data 828 * 829 * Returns 0 on success, or negative on failure. 830 */ > 831 int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, 832 u8 offset, u8 size, u8 *buf) 833 { 834 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 835 struct virtio_admin_cmd_legacy_rd_data *data; 836 struct scatterlist data_sg, result_sg; 837 struct virtio_admin_cmd cmd = {}; 838 int vf_id; 839 int ret; 840 841 if (!virtio_dev) 842 return -ENODEV; 843 844 vf_id = pci_iov_vf_id(pdev); 845 if (vf_id < 0) 846 return vf_id; 847 848 data = kzalloc(sizeof(*data), GFP_KERNEL); 849 if (!data) 850 return -ENOMEM; 851 852 data->offset = offset; 853 sg_init_one(&data_sg, data, sizeof(*data)); 854 sg_init_one(&result_sg, buf, size); 855 cmd.opcode = cpu_to_le16(opcode); 856 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 857 cmd.group_member_id = cpu_to_le64(vf_id + 1); 858 cmd.data_sg = &data_sg; 859 cmd.result_sg = &result_sg; 860 ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); 861 862 kfree(data); 863 return ret; 864 } 865 EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); 866 867 /* 868 * virtio_pci_admin_legacy_io_notify_info - Read the queue notification 869 * information for legacy interface 870 * @dev: VF pci_dev 871 * @req_bar_flags: requested bar flags 872 * @bar: on output the BAR number of the member device 873 * @bar_offset: on output the offset within bar 874 * 875 * Returns 0 on success, or negative on failure. 876 */ > 877 int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, 878 u8 req_bar_flags, u8 *bar, 879 u64 *bar_offset) 880 { 881 struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); 882 struct virtio_admin_cmd_notify_info_result *result; 883 struct virtio_admin_cmd cmd = {}; 884 struct scatterlist result_sg; 885 int vf_id; 886 int ret; 887 888 if (!virtio_dev) 889 return -ENODEV; 890 891 vf_id = pci_iov_vf_id(pdev); 892 if (vf_id < 0) 893 return vf_id; 894 895 result = kzalloc(sizeof(*result), GFP_KERNEL); 896 if (!result) 897 return -ENOMEM; 898 899 sg_init_one(&result_sg, result, sizeof(*result)); 900 cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); 901 cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); 902 cmd.group_member_id = cpu_to_le64(vf_id + 1); 903 cmd.result_sg = &result_sg; 904 ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); 905 if (!ret) { 906 struct virtio_admin_cmd_notify_info_data *entry; 907 int i; 908 909 ret = -ENOENT; 910 for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { 911 entry = &result->entries[i]; 912 if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) 913 break; 914 if (entry->flags != req_bar_flags) 915 continue; 916 *bar = entry->bar; 917 *bar_offset = le64_to_cpu(entry->offset); 918 ret = 0; 919 break; 920 } 921 } 922 923 kfree(result); 924 return ret; 925 } 926 EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); 927
On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: > Introduce APIs to execute legacy IO admin commands. > > It includes: list_query/use, io_legacy_read/write, > io_legacy_notify_info. > > Those APIs will be used by the next patches from this series. > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > --- > drivers/virtio/virtio_pci_common.c | 11 ++ > drivers/virtio/virtio_pci_common.h | 2 + > drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ > include/linux/virtio_pci_admin.h | 18 +++ > 4 files changed, 237 insertions(+) > create mode 100644 include/linux/virtio_pci_admin.h > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index 6b4766d5abe6..212d68401d2c 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { > .sriov_configure = virtio_pci_sriov_configure, > }; > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) > +{ > + struct virtio_pci_device *pf_vp_dev; > + > + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); > + if (IS_ERR(pf_vp_dev)) > + return NULL; > + > + return &pf_vp_dev->vdev; > +} > + > module_pci_driver(virtio_pci_driver); > > MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h > index a21b9ba01a60..2785e61ed668 100644 > --- a/drivers/virtio/virtio_pci_common.h > +++ b/drivers/virtio/virtio_pci_common.h > @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) > int virtio_pci_modern_probe(struct virtio_pci_device *); > void virtio_pci_modern_remove(struct virtio_pci_device *); > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); > + > #endif > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > index cc159a8e6c70..00b65e20b2f5 100644 > --- a/drivers/virtio/virtio_pci_modern.c > +++ b/drivers/virtio/virtio_pci_modern.c > @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) > vp_dev->del_vq(&vp_dev->admin_vq.info); > } > > +/* > + * virtio_pci_admin_list_query - Provides to driver list of commands > + * supported for the PCI VF. > + * @dev: VF pci_dev > + * @buf: buffer to hold the returned list > + * @buf_size: size of the given buffer > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist result_sg; > + > + if (!virtio_dev) > + return -ENODEV; > + > + sg_init_one(&result_sg, buf, buf_size); > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.result_sg = &result_sg; > + > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); > + > +/* > + * virtio_pci_admin_list_use - Provides to device list of commands > + * used for the PCI VF. > + * @dev: VF pci_dev > + * @buf: buffer which holds the list > + * @buf_size: size of the given buffer > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist data_sg; > + > + if (!virtio_dev) > + return -ENODEV; > + > + sg_init_one(&data_sg, buf, buf_size); > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.data_sg = &data_sg; > + > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); list commands are actually for a group, not for the VF. > + > +/* > + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device > + * @dev: VF pci_dev > + * @opcode: op code of the io write command opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? So please just add 2 APIs for this so users don't need to care. Could be wrappers around these two things. > + * @offset: starting byte offset within the registers to write to > + * @size: size of the data to write > + * @buf: buffer which holds the data > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd_legacy_wr_data *data; > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist data_sg; > + int vf_id; > + int ret; > + > + if (!virtio_dev) > + return -ENODEV; > + > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id < 0) > + return vf_id; > + > + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->offset = offset; > + memcpy(data->registers, buf, size); > + sg_init_one(&data_sg, data, sizeof(*data) + size); > + cmd.opcode = cpu_to_le16(opcode); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > + cmd.data_sg = &data_sg; > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > + > + kfree(data); > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); > + > +/* > + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device > + * @dev: VF pci_dev > + * @opcode: op code of the io read command > + * @offset: starting byte offset within the registers to read from > + * @size: size of the data to be read > + * @buf: buffer to hold the returned data > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd_legacy_rd_data *data; > + struct scatterlist data_sg, result_sg; > + struct virtio_admin_cmd cmd = {}; > + int vf_id; > + int ret; > + > + if (!virtio_dev) > + return -ENODEV; > + > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id < 0) > + return vf_id; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->offset = offset; > + sg_init_one(&data_sg, data, sizeof(*data)); > + sg_init_one(&result_sg, buf, size); > + cmd.opcode = cpu_to_le16(opcode); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > + cmd.data_sg = &data_sg; > + cmd.result_sg = &result_sg; > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > + > + kfree(data); > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); > + > +/* > + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification > + * information for legacy interface > + * @dev: VF pci_dev > + * @req_bar_flags: requested bar flags > + * @bar: on output the BAR number of the member device > + * @bar_offset: on output the offset within bar > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > + u8 req_bar_flags, u8 *bar, > + u64 *bar_offset) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd_notify_info_result *result; > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist result_sg; > + int vf_id; > + int ret; > + > + if (!virtio_dev) > + return -ENODEV; > + > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id < 0) > + return vf_id; > + > + result = kzalloc(sizeof(*result), GFP_KERNEL); > + if (!result) > + return -ENOMEM; > + > + sg_init_one(&result_sg, result, sizeof(*result)); > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > + cmd.result_sg = &result_sg; > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > + if (!ret) { > + struct virtio_admin_cmd_notify_info_data *entry; > + int i; > + > + ret = -ENOENT; > + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { > + entry = &result->entries[i]; > + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) > + break; > + if (entry->flags != req_bar_flags) > + continue; > + *bar = entry->bar; > + *bar_offset = le64_to_cpu(entry->offset); > + ret = 0; > + break; > + } > + } > + > + kfree(result); > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); > + > static const struct virtio_config_ops virtio_pci_config_nodev_ops = { > .get = NULL, > .set = NULL, > diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h > new file mode 100644 > index 000000000000..cb916a4bc1b1 > --- /dev/null > +++ b/include/linux/virtio_pci_admin.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H > +#define _LINUX_VIRTIO_PCI_ADMIN_H > + > +#include <linux/types.h> > +#include <linux/pci.h> > + > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf); > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf); > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > + u8 req_bar_flags, u8 *bar, > + u64 *bar_offset); > + > +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ > -- > 2.27.0
Re sending as previous reply was by mistake not in a text format. On 25/10/2023 0:01, Michael S. Tsirkin wrote: > On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: >> Introduce APIs to execute legacy IO admin commands. >> >> It includes: list_query/use, io_legacy_read/write, >> io_legacy_notify_info. >> >> Those APIs will be used by the next patches from this series. >> >> Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >> --- >> drivers/virtio/virtio_pci_common.c | 11 ++ >> drivers/virtio/virtio_pci_common.h | 2 + >> drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ >> include/linux/virtio_pci_admin.h | 18 +++ >> 4 files changed, 237 insertions(+) >> create mode 100644 include/linux/virtio_pci_admin.h >> >> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >> index 6b4766d5abe6..212d68401d2c 100644 >> --- a/drivers/virtio/virtio_pci_common.c >> +++ b/drivers/virtio/virtio_pci_common.c >> @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { >> .sriov_configure = virtio_pci_sriov_configure, >> }; >> >> +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) >> +{ >> + struct virtio_pci_device *pf_vp_dev; >> + >> + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); >> + if (IS_ERR(pf_vp_dev)) >> + return NULL; >> + >> + return &pf_vp_dev->vdev; >> +} >> + >> module_pci_driver(virtio_pci_driver); >> >> MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); >> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h >> index a21b9ba01a60..2785e61ed668 100644 >> --- a/drivers/virtio/virtio_pci_common.h >> +++ b/drivers/virtio/virtio_pci_common.h >> @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) >> int virtio_pci_modern_probe(struct virtio_pci_device *); >> void virtio_pci_modern_remove(struct virtio_pci_device *); >> >> +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); >> + >> #endif >> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c >> index cc159a8e6c70..00b65e20b2f5 100644 >> --- a/drivers/virtio/virtio_pci_modern.c >> +++ b/drivers/virtio/virtio_pci_modern.c >> @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) >> vp_dev->del_vq(&vp_dev->admin_vq.info); >> } >> >> +/* >> + * virtio_pci_admin_list_query - Provides to driver list of commands >> + * supported for the PCI VF. >> + * @dev: VF pci_dev >> + * @buf: buffer to hold the returned list >> + * @buf_size: size of the given buffer >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist result_sg; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + sg_init_one(&result_sg, buf, buf_size); >> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.result_sg = &result_sg; >> + >> + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); >> + >> +/* >> + * virtio_pci_admin_list_use - Provides to device list of commands >> + * used for the PCI VF. >> + * @dev: VF pci_dev >> + * @buf: buffer which holds the list >> + * @buf_size: size of the given buffer >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist data_sg; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + sg_init_one(&data_sg, buf, buf_size); >> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.data_sg = &data_sg; >> + >> + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); > list commands are actually for a group, not for the VF. The VF was given to let the function gets the PF from it. For now, the only existing 'group_type' in the spec is SRIOV, this is why we hard-coded it internally to match the VF PCI. Alternatively, We can change the API to get the PF and 'group_type' from the caller to better match future usage. However, this will require to export the virtio_pci_vf_get_pf_dev() API outside virtio-pci. Do you prefer to change to the latter option ? >> + >> +/* >> + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device >> + * @dev: VF pci_dev >> + * @opcode: op code of the io write command > opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE > or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? > > So please just add 2 APIs for this so users don't need to care. > Could be wrappers around these two things. > > OK. We'll export the below 2 APIs [1] which internally will call virtio_pci_admin_legacy_io_write() with the proper op code hard-coded. [1]virtio_pci_admin_legacy_device_io_write() virtio_pci_admin_legacy_common_io_write() Yishai > >> + * @offset: starting byte offset within the registers to write to >> + * @size: size of the data to write >> + * @buf: buffer which holds the data >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd_legacy_wr_data *data; >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist data_sg; >> + int vf_id; >> + int ret; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + vf_id = pci_iov_vf_id(pdev); >> + if (vf_id < 0) >> + return vf_id; >> + >> + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->offset = offset; >> + memcpy(data->registers, buf, size); >> + sg_init_one(&data_sg, data, sizeof(*data) + size); >> + cmd.opcode = cpu_to_le16(opcode); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >> + cmd.data_sg = &data_sg; >> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> + >> + kfree(data); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); >> + >> +/* >> + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device >> + * @dev: VF pci_dev >> + * @opcode: op code of the io read command >> + * @offset: starting byte offset within the registers to read from >> + * @size: size of the data to be read >> + * @buf: buffer to hold the returned data >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd_legacy_rd_data *data; >> + struct scatterlist data_sg, result_sg; >> + struct virtio_admin_cmd cmd = {}; >> + int vf_id; >> + int ret; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + vf_id = pci_iov_vf_id(pdev); >> + if (vf_id < 0) >> + return vf_id; >> + >> + data = kzalloc(sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->offset = offset; >> + sg_init_one(&data_sg, data, sizeof(*data)); >> + sg_init_one(&result_sg, buf, size); >> + cmd.opcode = cpu_to_le16(opcode); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >> + cmd.data_sg = &data_sg; >> + cmd.result_sg = &result_sg; >> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> + >> + kfree(data); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); >> + >> +/* >> + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification >> + * information for legacy interface >> + * @dev: VF pci_dev >> + * @req_bar_flags: requested bar flags >> + * @bar: on output the BAR number of the member device >> + * @bar_offset: on output the offset within bar >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, >> + u8 req_bar_flags, u8 *bar, >> + u64 *bar_offset) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd_notify_info_result *result; >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist result_sg; >> + int vf_id; >> + int ret; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + vf_id = pci_iov_vf_id(pdev); >> + if (vf_id < 0) >> + return vf_id; >> + >> + result = kzalloc(sizeof(*result), GFP_KERNEL); >> + if (!result) >> + return -ENOMEM; >> + >> + sg_init_one(&result_sg, result, sizeof(*result)); >> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >> + cmd.result_sg = &result_sg; >> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> + if (!ret) { >> + struct virtio_admin_cmd_notify_info_data *entry; >> + int i; >> + >> + ret = -ENOENT; >> + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { >> + entry = &result->entries[i]; >> + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) >> + break; >> + if (entry->flags != req_bar_flags) >> + continue; >> + *bar = entry->bar; >> + *bar_offset = le64_to_cpu(entry->offset); >> + ret = 0; >> + break; >> + } >> + } >> + >> + kfree(result); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); >> + >> static const struct virtio_config_ops virtio_pci_config_nodev_ops = { >> .get = NULL, >> .set = NULL, >> diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h >> new file mode 100644 >> index 000000000000..cb916a4bc1b1 >> --- /dev/null >> +++ b/include/linux/virtio_pci_admin.h >> @@ -0,0 +1,18 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H >> +#define _LINUX_VIRTIO_PCI_ADMIN_H >> + >> +#include <linux/types.h> >> +#include <linux/pci.h> >> + >> +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); >> +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); >> +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf); >> +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf); >> +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, >> + u8 req_bar_flags, u8 *bar, >> + u64 *bar_offset); >> + >> +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ >> -- >> 2.27.0
On Wed, Oct 25, 2023 at 12:18:32PM +0300, Yishai Hadas wrote: > On 25/10/2023 0:01, Michael S. Tsirkin wrote: > > On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: > > Introduce APIs to execute legacy IO admin commands. > > It includes: list_query/use, io_legacy_read/write, > io_legacy_notify_info. > > Those APIs will be used by the next patches from this series. > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > --- > drivers/virtio/virtio_pci_common.c | 11 ++ > drivers/virtio/virtio_pci_common.h | 2 + > drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ > include/linux/virtio_pci_admin.h | 18 +++ > 4 files changed, 237 insertions(+) > create mode 100644 include/linux/virtio_pci_admin.h > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index 6b4766d5abe6..212d68401d2c 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { > .sriov_configure = virtio_pci_sriov_configure, > }; > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) > +{ > + struct virtio_pci_device *pf_vp_dev; > + > + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); > + if (IS_ERR(pf_vp_dev)) > + return NULL; > + > + return &pf_vp_dev->vdev; > +} > + > module_pci_driver(virtio_pci_driver); > > MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h > index a21b9ba01a60..2785e61ed668 100644 > --- a/drivers/virtio/virtio_pci_common.h > +++ b/drivers/virtio/virtio_pci_common.h > @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) > int virtio_pci_modern_probe(struct virtio_pci_device *); > void virtio_pci_modern_remove(struct virtio_pci_device *); > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); > + > #endif > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > index cc159a8e6c70..00b65e20b2f5 100644 > --- a/drivers/virtio/virtio_pci_modern.c > +++ b/drivers/virtio/virtio_pci_modern.c > @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) > vp_dev->del_vq(&vp_dev->admin_vq.info); > } > > +/* > + * virtio_pci_admin_list_query - Provides to driver list of commands > + * supported for the PCI VF. > + * @dev: VF pci_dev > + * @buf: buffer to hold the returned list > + * @buf_size: size of the given buffer > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist result_sg; > + > + if (!virtio_dev) > + return -ENODEV; > + > + sg_init_one(&result_sg, buf, buf_size); > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.result_sg = &result_sg; > + > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); > + > +/* > + * virtio_pci_admin_list_use - Provides to device list of commands > + * used for the PCI VF. > + * @dev: VF pci_dev > + * @buf: buffer which holds the list > + * @buf_size: size of the given buffer > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist data_sg; > + > + if (!virtio_dev) > + return -ENODEV; > + > + sg_init_one(&data_sg, buf, buf_size); > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.data_sg = &data_sg; > + > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); > > list commands are actually for a group, not for the VF. > > The VF was given to let the function gets the PF from it. > > For now, the only existing 'group_type' in the spec is SRIOV, this is why we > hard-coded it internally to match the VF PCI. > > Alternatively, > We can change the API to get the PF and 'group_type' from the caller to better > match future usage. > However, this will require to export the virtio_pci_vf_get_pf_dev() API outside > virtio-pci. > > Do you prefer to change to the latter option ? No, there are several points I wanted to make but this was not one of them. First, for query, I was trying to suggest changing the comment. Something like: + * virtio_pci_admin_list_query - Provides to driver list of commands + * supported for the group including the given member device. + * @dev: member pci device. Second, I don't think using buf/size like this is necessary. For now we have a small number of commands just work with u64. Third, while list could be an OK API, the use API does not really work. If you call use with one set of parameters for one VF and another for another then they conflict do they not? So you need virtio core to do the list/use dance for you, save the list of commands on the PF (which again is just u64 for now) and vfio or vdpa or whatnot will just query that. I hope I'm being clear. > > > > + > +/* > + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device > + * @dev: VF pci_dev > + * @opcode: op code of the io write command > > opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE > or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? > > So please just add 2 APIs for this so users don't need to care. > Could be wrappers around these two things. > > > OK. > > We'll export the below 2 APIs [1] which internally will call > virtio_pci_admin_legacy_io_write() with the proper op code hard-coded. > > [1]virtio_pci_admin_legacy_device_io_write() > virtio_pci_admin_legacy_common_io_write() > > Yishai > Makes sense. > > > + * @offset: starting byte offset within the registers to write to > + * @size: size of the data to write > + * @buf: buffer which holds the data > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd_legacy_wr_data *data; > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist data_sg; > + int vf_id; > + int ret; > + > + if (!virtio_dev) > + return -ENODEV; > + > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id < 0) > + return vf_id; > + > + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->offset = offset; > + memcpy(data->registers, buf, size); > + sg_init_one(&data_sg, data, sizeof(*data) + size); > + cmd.opcode = cpu_to_le16(opcode); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > + cmd.data_sg = &data_sg; > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > + > + kfree(data); > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); > + > +/* > + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device > + * @dev: VF pci_dev > + * @opcode: op code of the io read command > + * @offset: starting byte offset within the registers to read from > + * @size: size of the data to be read > + * @buf: buffer to hold the returned data > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd_legacy_rd_data *data; > + struct scatterlist data_sg, result_sg; > + struct virtio_admin_cmd cmd = {}; > + int vf_id; > + int ret; > + > + if (!virtio_dev) > + return -ENODEV; > + > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id < 0) > + return vf_id; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->offset = offset; > + sg_init_one(&data_sg, data, sizeof(*data)); > + sg_init_one(&result_sg, buf, size); > + cmd.opcode = cpu_to_le16(opcode); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > + cmd.data_sg = &data_sg; > + cmd.result_sg = &result_sg; > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > + > + kfree(data); > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); > + > +/* > + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification > + * information for legacy interface > + * @dev: VF pci_dev > + * @req_bar_flags: requested bar flags > + * @bar: on output the BAR number of the member device > + * @bar_offset: on output the offset within bar > + * > + * Returns 0 on success, or negative on failure. > + */ > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > + u8 req_bar_flags, u8 *bar, > + u64 *bar_offset) > +{ > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > + struct virtio_admin_cmd_notify_info_result *result; > + struct virtio_admin_cmd cmd = {}; > + struct scatterlist result_sg; > + int vf_id; > + int ret; > + > + if (!virtio_dev) > + return -ENODEV; > + > + vf_id = pci_iov_vf_id(pdev); > + if (vf_id < 0) > + return vf_id; > + > + result = kzalloc(sizeof(*result), GFP_KERNEL); > + if (!result) > + return -ENOMEM; > + > + sg_init_one(&result_sg, result, sizeof(*result)); > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > + cmd.result_sg = &result_sg; > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > + if (!ret) { > + struct virtio_admin_cmd_notify_info_data *entry; > + int i; > + > + ret = -ENOENT; > + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { > + entry = &result->entries[i]; > + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) > + break; > + if (entry->flags != req_bar_flags) > + continue; > + *bar = entry->bar; > + *bar_offset = le64_to_cpu(entry->offset); > + ret = 0; > + break; > + } > + } > + > + kfree(result); > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); > + > static const struct virtio_config_ops virtio_pci_config_nodev_ops = { > .get = NULL, > .set = NULL, > diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h > new file mode 100644 > index 000000000000..cb916a4bc1b1 > --- /dev/null > +++ b/include/linux/virtio_pci_admin.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H > +#define _LINUX_VIRTIO_PCI_ADMIN_H > + > +#include <linux/types.h> > +#include <linux/pci.h> > + > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf); > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > + u8 offset, u8 size, u8 *buf); > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > + u8 req_bar_flags, u8 *bar, > + u64 *bar_offset); > + > +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ > -- > 2.27.0 > >
On 25/10/2023 13:17, Michael S. Tsirkin wrote: > On Wed, Oct 25, 2023 at 12:18:32PM +0300, Yishai Hadas wrote: >> On 25/10/2023 0:01, Michael S. Tsirkin wrote: >> >> On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: >> >> Introduce APIs to execute legacy IO admin commands. >> >> It includes: list_query/use, io_legacy_read/write, >> io_legacy_notify_info. >> >> Those APIs will be used by the next patches from this series. >> >> Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >> --- >> drivers/virtio/virtio_pci_common.c | 11 ++ >> drivers/virtio/virtio_pci_common.h | 2 + >> drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ >> include/linux/virtio_pci_admin.h | 18 +++ >> 4 files changed, 237 insertions(+) >> create mode 100644 include/linux/virtio_pci_admin.h >> >> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >> index 6b4766d5abe6..212d68401d2c 100644 >> --- a/drivers/virtio/virtio_pci_common.c >> +++ b/drivers/virtio/virtio_pci_common.c >> @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { >> .sriov_configure = virtio_pci_sriov_configure, >> }; >> >> +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) >> +{ >> + struct virtio_pci_device *pf_vp_dev; >> + >> + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); >> + if (IS_ERR(pf_vp_dev)) >> + return NULL; >> + >> + return &pf_vp_dev->vdev; >> +} >> + >> module_pci_driver(virtio_pci_driver); >> >> MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); >> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h >> index a21b9ba01a60..2785e61ed668 100644 >> --- a/drivers/virtio/virtio_pci_common.h >> +++ b/drivers/virtio/virtio_pci_common.h >> @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) >> int virtio_pci_modern_probe(struct virtio_pci_device *); >> void virtio_pci_modern_remove(struct virtio_pci_device *); >> >> +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); >> + >> #endif >> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c >> index cc159a8e6c70..00b65e20b2f5 100644 >> --- a/drivers/virtio/virtio_pci_modern.c >> +++ b/drivers/virtio/virtio_pci_modern.c >> @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) >> vp_dev->del_vq(&vp_dev->admin_vq.info); >> } >> >> +/* >> + * virtio_pci_admin_list_query - Provides to driver list of commands >> + * supported for the PCI VF. >> + * @dev: VF pci_dev >> + * @buf: buffer to hold the returned list >> + * @buf_size: size of the given buffer >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist result_sg; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + sg_init_one(&result_sg, buf, buf_size); >> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.result_sg = &result_sg; >> + >> + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); >> + >> +/* >> + * virtio_pci_admin_list_use - Provides to device list of commands >> + * used for the PCI VF. >> + * @dev: VF pci_dev >> + * @buf: buffer which holds the list >> + * @buf_size: size of the given buffer >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist data_sg; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + sg_init_one(&data_sg, buf, buf_size); >> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.data_sg = &data_sg; >> + >> + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); >> >> list commands are actually for a group, not for the VF. >> >> The VF was given to let the function gets the PF from it. >> >> For now, the only existing 'group_type' in the spec is SRIOV, this is why we >> hard-coded it internally to match the VF PCI. >> >> Alternatively, >> We can change the API to get the PF and 'group_type' from the caller to better >> match future usage. >> However, this will require to export the virtio_pci_vf_get_pf_dev() API outside >> virtio-pci. >> >> Do you prefer to change to the latter option ? > No, there are several points I wanted to make but this > was not one of them. > > First, for query, I was trying to suggest changing the comment. > Something like: > + * virtio_pci_admin_list_query - Provides to driver list of commands > + * supported for the group including the given member device. > + * @dev: member pci device. Following your suggestion below, to issue inside virtio the query/use and keep its data internally (i.e. on the 'admin_queue' context). We may suggest the below API for the upper-layers (e.g. vfio) to be exported. bool virtio_pci_admin_supported_cmds(struct pci_dev *pdev, u64 cmds) It will find the PF from the VF and internally will check on the 'admin_queue' context whether the given 'cmds' input is supported. Its output will be true/false. Makes sense ? > > > > Second, I don't think using buf/size like this is necessary. > For now we have a small number of commands just work with u64. OK, just keep in mind that upon issuing the command towards the controller this still needs to be an allocated u64 data on the heap to work properly. > > > Third, while list could be an OK API, the use API does not > really work. If you call use with one set of parameters for > one VF and another for another then they conflict do they not? > > So you need virtio core to do the list/use dance for you, > save the list of commands on the PF (which again is just u64 for now) > and vfio or vdpa or whatnot will just query that. > I hope I'm being clear. In that case the virtio_pci_admin_list_query() and virtio_pci_admin_list_use() won't be exported any more and will be static in virtio-pci. They will be called internally as part of activating the admin_queue and will simply get struct virtio_device* (the PF) instead of struct pci_dev *pdev. > > >> >> >> + >> +/* >> + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device >> + * @dev: VF pci_dev >> + * @opcode: op code of the io write command >> >> opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE >> or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? >> >> So please just add 2 APIs for this so users don't need to care. >> Could be wrappers around these two things. >> >> >> OK. >> >> We'll export the below 2 APIs [1] which internally will call >> virtio_pci_admin_legacy_io_write() with the proper op code hard-coded. >> >> [1]virtio_pci_admin_legacy_device_io_write() >> virtio_pci_admin_legacy_common_io_write() >> >> Yishai >> > Makes sense. > OK, we may do the same split for the 'legacy_io_read' commands to be symmetric with the 'legacy_io_write', right ? Yishai >> >> + * @offset: starting byte offset within the registers to write to >> + * @size: size of the data to write >> + * @buf: buffer which holds the data >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd_legacy_wr_data *data; >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist data_sg; >> + int vf_id; >> + int ret; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + vf_id = pci_iov_vf_id(pdev); >> + if (vf_id < 0) >> + return vf_id; >> + >> + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->offset = offset; >> + memcpy(data->registers, buf, size); >> + sg_init_one(&data_sg, data, sizeof(*data) + size); >> + cmd.opcode = cpu_to_le16(opcode); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >> + cmd.data_sg = &data_sg; >> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> + >> + kfree(data); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); >> + >> +/* >> + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device >> + * @dev: VF pci_dev >> + * @opcode: op code of the io read command >> + * @offset: starting byte offset within the registers to read from >> + * @size: size of the data to be read >> + * @buf: buffer to hold the returned data >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd_legacy_rd_data *data; >> + struct scatterlist data_sg, result_sg; >> + struct virtio_admin_cmd cmd = {}; >> + int vf_id; >> + int ret; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + vf_id = pci_iov_vf_id(pdev); >> + if (vf_id < 0) >> + return vf_id; >> + >> + data = kzalloc(sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->offset = offset; >> + sg_init_one(&data_sg, data, sizeof(*data)); >> + sg_init_one(&result_sg, buf, size); >> + cmd.opcode = cpu_to_le16(opcode); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >> + cmd.data_sg = &data_sg; >> + cmd.result_sg = &result_sg; >> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> + >> + kfree(data); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); >> + >> +/* >> + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification >> + * information for legacy interface >> + * @dev: VF pci_dev >> + * @req_bar_flags: requested bar flags >> + * @bar: on output the BAR number of the member device >> + * @bar_offset: on output the offset within bar >> + * >> + * Returns 0 on success, or negative on failure. >> + */ >> +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, >> + u8 req_bar_flags, u8 *bar, >> + u64 *bar_offset) >> +{ >> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >> + struct virtio_admin_cmd_notify_info_result *result; >> + struct virtio_admin_cmd cmd = {}; >> + struct scatterlist result_sg; >> + int vf_id; >> + int ret; >> + >> + if (!virtio_dev) >> + return -ENODEV; >> + >> + vf_id = pci_iov_vf_id(pdev); >> + if (vf_id < 0) >> + return vf_id; >> + >> + result = kzalloc(sizeof(*result), GFP_KERNEL); >> + if (!result) >> + return -ENOMEM; >> + >> + sg_init_one(&result_sg, result, sizeof(*result)); >> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); >> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >> + cmd.result_sg = &result_sg; >> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >> + if (!ret) { >> + struct virtio_admin_cmd_notify_info_data *entry; >> + int i; >> + >> + ret = -ENOENT; >> + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { >> + entry = &result->entries[i]; >> + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) >> + break; >> + if (entry->flags != req_bar_flags) >> + continue; >> + *bar = entry->bar; >> + *bar_offset = le64_to_cpu(entry->offset); >> + ret = 0; >> + break; >> + } >> + } >> + >> + kfree(result); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); >> + >> static const struct virtio_config_ops virtio_pci_config_nodev_ops = { >> .get = NULL, >> .set = NULL, >> diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h >> new file mode 100644 >> index 000000000000..cb916a4bc1b1 >> --- /dev/null >> +++ b/include/linux/virtio_pci_admin.h >> @@ -0,0 +1,18 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H >> +#define _LINUX_VIRTIO_PCI_ADMIN_H >> + >> +#include <linux/types.h> >> +#include <linux/pci.h> >> + >> +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); >> +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); >> +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf); >> +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, >> + u8 offset, u8 size, u8 *buf); >> +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, >> + u8 req_bar_flags, u8 *bar, >> + u64 *bar_offset); >> + >> +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ >> -- >> 2.27.0 >> >>
On Wed, Oct 25, 2023 at 04:00:43PM +0300, Yishai Hadas wrote: > On 25/10/2023 13:17, Michael S. Tsirkin wrote: > > On Wed, Oct 25, 2023 at 12:18:32PM +0300, Yishai Hadas wrote: > > > On 25/10/2023 0:01, Michael S. Tsirkin wrote: > > > > > > On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: > > > > > > Introduce APIs to execute legacy IO admin commands. > > > > > > It includes: list_query/use, io_legacy_read/write, > > > io_legacy_notify_info. > > > > > > Those APIs will be used by the next patches from this series. > > > > > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > > > --- > > > drivers/virtio/virtio_pci_common.c | 11 ++ > > > drivers/virtio/virtio_pci_common.h | 2 + > > > drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ > > > include/linux/virtio_pci_admin.h | 18 +++ > > > 4 files changed, 237 insertions(+) > > > create mode 100644 include/linux/virtio_pci_admin.h > > > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > > index 6b4766d5abe6..212d68401d2c 100644 > > > --- a/drivers/virtio/virtio_pci_common.c > > > +++ b/drivers/virtio/virtio_pci_common.c > > > @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { > > > .sriov_configure = virtio_pci_sriov_configure, > > > }; > > > > > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) > > > +{ > > > + struct virtio_pci_device *pf_vp_dev; > > > + > > > + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); > > > + if (IS_ERR(pf_vp_dev)) > > > + return NULL; > > > + > > > + return &pf_vp_dev->vdev; > > > +} > > > + > > > module_pci_driver(virtio_pci_driver); > > > > > > MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h > > > index a21b9ba01a60..2785e61ed668 100644 > > > --- a/drivers/virtio/virtio_pci_common.h > > > +++ b/drivers/virtio/virtio_pci_common.h > > > @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) > > > int virtio_pci_modern_probe(struct virtio_pci_device *); > > > void virtio_pci_modern_remove(struct virtio_pci_device *); > > > > > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); > > > + > > > #endif > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > > > index cc159a8e6c70..00b65e20b2f5 100644 > > > --- a/drivers/virtio/virtio_pci_modern.c > > > +++ b/drivers/virtio/virtio_pci_modern.c > > > @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) > > > vp_dev->del_vq(&vp_dev->admin_vq.info); > > > } > > > > > > +/* > > > + * virtio_pci_admin_list_query - Provides to driver list of commands > > > + * supported for the PCI VF. > > > + * @dev: VF pci_dev > > > + * @buf: buffer to hold the returned list > > > + * @buf_size: size of the given buffer > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist result_sg; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + sg_init_one(&result_sg, buf, buf_size); > > > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.result_sg = &result_sg; > > > + > > > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); > > > + > > > +/* > > > + * virtio_pci_admin_list_use - Provides to device list of commands > > > + * used for the PCI VF. > > > + * @dev: VF pci_dev > > > + * @buf: buffer which holds the list > > > + * @buf_size: size of the given buffer > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist data_sg; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + sg_init_one(&data_sg, buf, buf_size); > > > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.data_sg = &data_sg; > > > + > > > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); > > > > > > list commands are actually for a group, not for the VF. > > > > > > The VF was given to let the function gets the PF from it. > > > > > > For now, the only existing 'group_type' in the spec is SRIOV, this is why we > > > hard-coded it internally to match the VF PCI. > > > > > > Alternatively, > > > We can change the API to get the PF and 'group_type' from the caller to better > > > match future usage. > > > However, this will require to export the virtio_pci_vf_get_pf_dev() API outside > > > virtio-pci. > > > > > > Do you prefer to change to the latter option ? > > No, there are several points I wanted to make but this > > was not one of them. > > > > First, for query, I was trying to suggest changing the comment. > > Something like: > > + * virtio_pci_admin_list_query - Provides to driver list of commands > > + * supported for the group including the given member device. > > + * @dev: member pci device. > > Following your suggestion below, to issue inside virtio the query/use and > keep its data internally (i.e. on the 'admin_queue' context). > > We may suggest the below API for the upper-layers (e.g. vfio) to be > exported. > > bool virtio_pci_admin_supported_cmds(struct pci_dev *pdev, u64 cmds) > > It will find the PF from the VF and internally will check on the > 'admin_queue' context whether the given 'cmds' input is supported. > > Its output will be true/false. > > Makes sense ? I think I'd just return the commands. But not a big deal. > > > > > > > > Second, I don't think using buf/size like this is necessary. > > For now we have a small number of commands just work with u64. > OK, just keep in mind that upon issuing the command towards the controller > this still needs to be an allocated u64 data on the heap to work properly. > > > > > > Third, while list could be an OK API, the use API does not > > really work. If you call use with one set of parameters for > > one VF and another for another then they conflict do they not? > > > > So you need virtio core to do the list/use dance for you, > > save the list of commands on the PF (which again is just u64 for now) > > and vfio or vdpa or whatnot will just query that. > > I hope I'm being clear. > > In that case the virtio_pci_admin_list_query() and > virtio_pci_admin_list_use() won't be exported any more and will be static in > virtio-pci. > > They will be called internally as part of activating the admin_queue and > will simply get struct virtio_device* (the PF) instead of struct pci_dev > *pdev. > > > > > > > > > > > > > > + > > > +/* > > > + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device > > > + * @dev: VF pci_dev > > > + * @opcode: op code of the io write command > > > > > > opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE > > > or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? > > > > > > So please just add 2 APIs for this so users don't need to care. > > > Could be wrappers around these two things. > > > > > > > > > OK. > > > > > > We'll export the below 2 APIs [1] which internally will call > > > virtio_pci_admin_legacy_io_write() with the proper op code hard-coded. > > > > > > [1]virtio_pci_admin_legacy_device_io_write() > > > virtio_pci_admin_legacy_common_io_write() > > > > > > Yishai > > > > > Makes sense. > > OK, we may do the same split for the 'legacy_io_read' commands to be > symmetric with the 'legacy_io_write', right ? > > Yishai > > > > > > > + * @offset: starting byte offset within the registers to write to > > > + * @size: size of the data to write > > > + * @buf: buffer which holds the data > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd_legacy_wr_data *data; > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist data_sg; > > > + int vf_id; > > > + int ret; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + vf_id = pci_iov_vf_id(pdev); > > > + if (vf_id < 0) > > > + return vf_id; > > > + > > > + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); > > > + if (!data) > > > + return -ENOMEM; > > > + > > > + data->offset = offset; > > > + memcpy(data->registers, buf, size); > > > + sg_init_one(&data_sg, data, sizeof(*data) + size); > > > + cmd.opcode = cpu_to_le16(opcode); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > > > + cmd.data_sg = &data_sg; > > > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > + > > > + kfree(data); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); > > > + > > > +/* > > > + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device > > > + * @dev: VF pci_dev > > > + * @opcode: op code of the io read command > > > + * @offset: starting byte offset within the registers to read from > > > + * @size: size of the data to be read > > > + * @buf: buffer to hold the returned data > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd_legacy_rd_data *data; > > > + struct scatterlist data_sg, result_sg; > > > + struct virtio_admin_cmd cmd = {}; > > > + int vf_id; > > > + int ret; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + vf_id = pci_iov_vf_id(pdev); > > > + if (vf_id < 0) > > > + return vf_id; > > > + > > > + data = kzalloc(sizeof(*data), GFP_KERNEL); > > > + if (!data) > > > + return -ENOMEM; > > > + > > > + data->offset = offset; > > > + sg_init_one(&data_sg, data, sizeof(*data)); > > > + sg_init_one(&result_sg, buf, size); > > > + cmd.opcode = cpu_to_le16(opcode); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > > > + cmd.data_sg = &data_sg; > > > + cmd.result_sg = &result_sg; > > > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > + > > > + kfree(data); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); > > > + > > > +/* > > > + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification > > > + * information for legacy interface > > > + * @dev: VF pci_dev > > > + * @req_bar_flags: requested bar flags > > > + * @bar: on output the BAR number of the member device > > > + * @bar_offset: on output the offset within bar > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > > > + u8 req_bar_flags, u8 *bar, > > > + u64 *bar_offset) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd_notify_info_result *result; > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist result_sg; > > > + int vf_id; > > > + int ret; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + vf_id = pci_iov_vf_id(pdev); > > > + if (vf_id < 0) > > > + return vf_id; > > > + > > > + result = kzalloc(sizeof(*result), GFP_KERNEL); > > > + if (!result) > > > + return -ENOMEM; > > > + > > > + sg_init_one(&result_sg, result, sizeof(*result)); > > > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > > > + cmd.result_sg = &result_sg; > > > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > + if (!ret) { > > > + struct virtio_admin_cmd_notify_info_data *entry; > > > + int i; > > > + > > > + ret = -ENOENT; > > > + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { > > > + entry = &result->entries[i]; > > > + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) > > > + break; > > > + if (entry->flags != req_bar_flags) > > > + continue; > > > + *bar = entry->bar; > > > + *bar_offset = le64_to_cpu(entry->offset); > > > + ret = 0; > > > + break; > > > + } > > > + } > > > + > > > + kfree(result); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); > > > + > > > static const struct virtio_config_ops virtio_pci_config_nodev_ops = { > > > .get = NULL, > > > .set = NULL, > > > diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h > > > new file mode 100644 > > > index 000000000000..cb916a4bc1b1 > > > --- /dev/null > > > +++ b/include/linux/virtio_pci_admin.h > > > @@ -0,0 +1,18 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H > > > +#define _LINUX_VIRTIO_PCI_ADMIN_H > > > + > > > +#include <linux/types.h> > > > +#include <linux/pci.h> > > > + > > > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); > > > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); > > > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf); > > > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf); > > > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > > > + u8 req_bar_flags, u8 *bar, > > > + u64 *bar_offset); > > > + > > > +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ > > > -- > > > 2.27.0 > > > > > >
On Wed, Oct 25, 2023 at 04:00:43PM +0300, Yishai Hadas wrote: > On 25/10/2023 13:17, Michael S. Tsirkin wrote: > > On Wed, Oct 25, 2023 at 12:18:32PM +0300, Yishai Hadas wrote: > > > On 25/10/2023 0:01, Michael S. Tsirkin wrote: > > > > > > On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: > > > > > > Introduce APIs to execute legacy IO admin commands. > > > > > > It includes: list_query/use, io_legacy_read/write, > > > io_legacy_notify_info. > > > > > > Those APIs will be used by the next patches from this series. > > > > > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > > > --- > > > drivers/virtio/virtio_pci_common.c | 11 ++ > > > drivers/virtio/virtio_pci_common.h | 2 + > > > drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ > > > include/linux/virtio_pci_admin.h | 18 +++ > > > 4 files changed, 237 insertions(+) > > > create mode 100644 include/linux/virtio_pci_admin.h > > > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > > index 6b4766d5abe6..212d68401d2c 100644 > > > --- a/drivers/virtio/virtio_pci_common.c > > > +++ b/drivers/virtio/virtio_pci_common.c > > > @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { > > > .sriov_configure = virtio_pci_sriov_configure, > > > }; > > > > > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) > > > +{ > > > + struct virtio_pci_device *pf_vp_dev; > > > + > > > + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); > > > + if (IS_ERR(pf_vp_dev)) > > > + return NULL; > > > + > > > + return &pf_vp_dev->vdev; > > > +} > > > + > > > module_pci_driver(virtio_pci_driver); > > > > > > MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h > > > index a21b9ba01a60..2785e61ed668 100644 > > > --- a/drivers/virtio/virtio_pci_common.h > > > +++ b/drivers/virtio/virtio_pci_common.h > > > @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) > > > int virtio_pci_modern_probe(struct virtio_pci_device *); > > > void virtio_pci_modern_remove(struct virtio_pci_device *); > > > > > > +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); > > > + > > > #endif > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > > > index cc159a8e6c70..00b65e20b2f5 100644 > > > --- a/drivers/virtio/virtio_pci_modern.c > > > +++ b/drivers/virtio/virtio_pci_modern.c > > > @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) > > > vp_dev->del_vq(&vp_dev->admin_vq.info); > > > } > > > > > > +/* > > > + * virtio_pci_admin_list_query - Provides to driver list of commands > > > + * supported for the PCI VF. > > > + * @dev: VF pci_dev > > > + * @buf: buffer to hold the returned list > > > + * @buf_size: size of the given buffer > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist result_sg; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + sg_init_one(&result_sg, buf, buf_size); > > > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.result_sg = &result_sg; > > > + > > > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); > > > + > > > +/* > > > + * virtio_pci_admin_list_use - Provides to device list of commands > > > + * used for the PCI VF. > > > + * @dev: VF pci_dev > > > + * @buf: buffer which holds the list > > > + * @buf_size: size of the given buffer > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist data_sg; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + sg_init_one(&data_sg, buf, buf_size); > > > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.data_sg = &data_sg; > > > + > > > + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); > > > > > > list commands are actually for a group, not for the VF. > > > > > > The VF was given to let the function gets the PF from it. > > > > > > For now, the only existing 'group_type' in the spec is SRIOV, this is why we > > > hard-coded it internally to match the VF PCI. > > > > > > Alternatively, > > > We can change the API to get the PF and 'group_type' from the caller to better > > > match future usage. > > > However, this will require to export the virtio_pci_vf_get_pf_dev() API outside > > > virtio-pci. > > > > > > Do you prefer to change to the latter option ? > > No, there are several points I wanted to make but this > > was not one of them. > > > > First, for query, I was trying to suggest changing the comment. > > Something like: > > + * virtio_pci_admin_list_query - Provides to driver list of commands > > + * supported for the group including the given member device. > > + * @dev: member pci device. > > Following your suggestion below, to issue inside virtio the query/use and > keep its data internally (i.e. on the 'admin_queue' context). > > We may suggest the below API for the upper-layers (e.g. vfio) to be > exported. > > bool virtio_pci_admin_supported_cmds(struct pci_dev *pdev, u64 cmds) > > It will find the PF from the VF and internally will check on the > 'admin_queue' context whether the given 'cmds' input is supported. > > Its output will be true/false. > > Makes sense ? > > > > > > > > > Second, I don't think using buf/size like this is necessary. > > For now we have a small number of commands just work with u64. > OK, just keep in mind that upon issuing the command towards the controller > this still needs to be an allocated u64 data on the heap to work properly. > > > > > > Third, while list could be an OK API, the use API does not > > really work. If you call use with one set of parameters for > > one VF and another for another then they conflict do they not? > > > > So you need virtio core to do the list/use dance for you, > > save the list of commands on the PF (which again is just u64 for now) > > and vfio or vdpa or whatnot will just query that. > > I hope I'm being clear. > > In that case the virtio_pci_admin_list_query() and > virtio_pci_admin_list_use() won't be exported any more and will be static in > virtio-pci. > > They will be called internally as part of activating the admin_queue and > will simply get struct virtio_device* (the PF) instead of struct pci_dev > *pdev. Yes - I think some kind of API will be needed to setup/cleanup. Then 1st call to setup would do the list/use dance? some ref counting? And maybe the API should just be bool virtio_pci_admin_has_legacy_io() > > > > > > > > > > > > > + > > > +/* > > > + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device > > > + * @dev: VF pci_dev > > > + * @opcode: op code of the io write command > > > > > > opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE > > > or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? > > > > > > So please just add 2 APIs for this so users don't need to care. > > > Could be wrappers around these two things. > > > > > > > > > OK. > > > > > > We'll export the below 2 APIs [1] which internally will call > > > virtio_pci_admin_legacy_io_write() with the proper op code hard-coded. > > > > > > [1]virtio_pci_admin_legacy_device_io_write() > > > virtio_pci_admin_legacy_common_io_write() > > > > > > Yishai > > > > > Makes sense. > > OK, we may do the same split for the 'legacy_io_read' commands to be > symmetric with the 'legacy_io_write', right ? > > Yishai makes sense. > > > > > > + * @offset: starting byte offset within the registers to write to > > > + * @size: size of the data to write > > > + * @buf: buffer which holds the data > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd_legacy_wr_data *data; > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist data_sg; > > > + int vf_id; > > > + int ret; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + vf_id = pci_iov_vf_id(pdev); > > > + if (vf_id < 0) > > > + return vf_id; > > > + > > > + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); > > > + if (!data) > > > + return -ENOMEM; > > > + > > > + data->offset = offset; > > > + memcpy(data->registers, buf, size); > > > + sg_init_one(&data_sg, data, sizeof(*data) + size); > > > + cmd.opcode = cpu_to_le16(opcode); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > > > + cmd.data_sg = &data_sg; > > > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > + > > > + kfree(data); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); > > > + > > > +/* > > > + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device > > > + * @dev: VF pci_dev > > > + * @opcode: op code of the io read command > > > + * @offset: starting byte offset within the registers to read from > > > + * @size: size of the data to be read > > > + * @buf: buffer to hold the returned data > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd_legacy_rd_data *data; > > > + struct scatterlist data_sg, result_sg; > > > + struct virtio_admin_cmd cmd = {}; > > > + int vf_id; > > > + int ret; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + vf_id = pci_iov_vf_id(pdev); > > > + if (vf_id < 0) > > > + return vf_id; > > > + > > > + data = kzalloc(sizeof(*data), GFP_KERNEL); > > > + if (!data) > > > + return -ENOMEM; > > > + > > > + data->offset = offset; > > > + sg_init_one(&data_sg, data, sizeof(*data)); > > > + sg_init_one(&result_sg, buf, size); > > > + cmd.opcode = cpu_to_le16(opcode); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > > > + cmd.data_sg = &data_sg; > > > + cmd.result_sg = &result_sg; > > > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > + > > > + kfree(data); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); > > > + > > > +/* > > > + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification > > > + * information for legacy interface > > > + * @dev: VF pci_dev > > > + * @req_bar_flags: requested bar flags > > > + * @bar: on output the BAR number of the member device > > > + * @bar_offset: on output the offset within bar > > > + * > > > + * Returns 0 on success, or negative on failure. > > > + */ > > > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > > > + u8 req_bar_flags, u8 *bar, > > > + u64 *bar_offset) > > > +{ > > > + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); > > > + struct virtio_admin_cmd_notify_info_result *result; > > > + struct virtio_admin_cmd cmd = {}; > > > + struct scatterlist result_sg; > > > + int vf_id; > > > + int ret; > > > + > > > + if (!virtio_dev) > > > + return -ENODEV; > > > + > > > + vf_id = pci_iov_vf_id(pdev); > > > + if (vf_id < 0) > > > + return vf_id; > > > + > > > + result = kzalloc(sizeof(*result), GFP_KERNEL); > > > + if (!result) > > > + return -ENOMEM; > > > + > > > + sg_init_one(&result_sg, result, sizeof(*result)); > > > + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); > > > + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); > > > + cmd.group_member_id = cpu_to_le64(vf_id + 1); > > > + cmd.result_sg = &result_sg; > > > + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); > > > + if (!ret) { > > > + struct virtio_admin_cmd_notify_info_data *entry; > > > + int i; > > > + > > > + ret = -ENOENT; > > > + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { > > > + entry = &result->entries[i]; > > > + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) > > > + break; > > > + if (entry->flags != req_bar_flags) > > > + continue; > > > + *bar = entry->bar; > > > + *bar_offset = le64_to_cpu(entry->offset); > > > + ret = 0; > > > + break; > > > + } > > > + } > > > + > > > + kfree(result); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); > > > + > > > static const struct virtio_config_ops virtio_pci_config_nodev_ops = { > > > .get = NULL, > > > .set = NULL, > > > diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h > > > new file mode 100644 > > > index 000000000000..cb916a4bc1b1 > > > --- /dev/null > > > +++ b/include/linux/virtio_pci_admin.h > > > @@ -0,0 +1,18 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H > > > +#define _LINUX_VIRTIO_PCI_ADMIN_H > > > + > > > +#include <linux/types.h> > > > +#include <linux/pci.h> > > > + > > > +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); > > > +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); > > > +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf); > > > +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, > > > + u8 offset, u8 size, u8 *buf); > > > +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, > > > + u8 req_bar_flags, u8 *bar, > > > + u64 *bar_offset); > > > + > > > +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ > > > -- > > > 2.27.0 > > > > > >
On 25/10/2023 16:44, Michael S. Tsirkin wrote: > On Wed, Oct 25, 2023 at 04:00:43PM +0300, Yishai Hadas wrote: >> On 25/10/2023 13:17, Michael S. Tsirkin wrote: >>> On Wed, Oct 25, 2023 at 12:18:32PM +0300, Yishai Hadas wrote: >>>> On 25/10/2023 0:01, Michael S. Tsirkin wrote: >>>> >>>> On Tue, Oct 17, 2023 at 04:42:14PM +0300, Yishai Hadas wrote: >>>> >>>> Introduce APIs to execute legacy IO admin commands. >>>> >>>> It includes: list_query/use, io_legacy_read/write, >>>> io_legacy_notify_info. >>>> >>>> Those APIs will be used by the next patches from this series. >>>> >>>> Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >>>> --- >>>> drivers/virtio/virtio_pci_common.c | 11 ++ >>>> drivers/virtio/virtio_pci_common.h | 2 + >>>> drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ >>>> include/linux/virtio_pci_admin.h | 18 +++ >>>> 4 files changed, 237 insertions(+) >>>> create mode 100644 include/linux/virtio_pci_admin.h >>>> >>>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >>>> index 6b4766d5abe6..212d68401d2c 100644 >>>> --- a/drivers/virtio/virtio_pci_common.c >>>> +++ b/drivers/virtio/virtio_pci_common.c >>>> @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { >>>> .sriov_configure = virtio_pci_sriov_configure, >>>> }; >>>> >>>> +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) >>>> +{ >>>> + struct virtio_pci_device *pf_vp_dev; >>>> + >>>> + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); >>>> + if (IS_ERR(pf_vp_dev)) >>>> + return NULL; >>>> + >>>> + return &pf_vp_dev->vdev; >>>> +} >>>> + >>>> module_pci_driver(virtio_pci_driver); >>>> >>>> MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); >>>> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h >>>> index a21b9ba01a60..2785e61ed668 100644 >>>> --- a/drivers/virtio/virtio_pci_common.h >>>> +++ b/drivers/virtio/virtio_pci_common.h >>>> @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) >>>> int virtio_pci_modern_probe(struct virtio_pci_device *); >>>> void virtio_pci_modern_remove(struct virtio_pci_device *); >>>> >>>> +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); >>>> + >>>> #endif >>>> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c >>>> index cc159a8e6c70..00b65e20b2f5 100644 >>>> --- a/drivers/virtio/virtio_pci_modern.c >>>> +++ b/drivers/virtio/virtio_pci_modern.c >>>> @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) >>>> vp_dev->del_vq(&vp_dev->admin_vq.info); >>>> } >>>> >>>> +/* >>>> + * virtio_pci_admin_list_query - Provides to driver list of commands >>>> + * supported for the PCI VF. >>>> + * @dev: VF pci_dev >>>> + * @buf: buffer to hold the returned list >>>> + * @buf_size: size of the given buffer >>>> + * >>>> + * Returns 0 on success, or negative on failure. >>>> + */ >>>> +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) >>>> +{ >>>> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >>>> + struct virtio_admin_cmd cmd = {}; >>>> + struct scatterlist result_sg; >>>> + >>>> + if (!virtio_dev) >>>> + return -ENODEV; >>>> + >>>> + sg_init_one(&result_sg, buf, buf_size); >>>> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); >>>> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >>>> + cmd.result_sg = &result_sg; >>>> + >>>> + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); >>>> +} >>>> +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); >>>> + >>>> +/* >>>> + * virtio_pci_admin_list_use - Provides to device list of commands >>>> + * used for the PCI VF. >>>> + * @dev: VF pci_dev >>>> + * @buf: buffer which holds the list >>>> + * @buf_size: size of the given buffer >>>> + * >>>> + * Returns 0 on success, or negative on failure. >>>> + */ >>>> +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) >>>> +{ >>>> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >>>> + struct virtio_admin_cmd cmd = {}; >>>> + struct scatterlist data_sg; >>>> + >>>> + if (!virtio_dev) >>>> + return -ENODEV; >>>> + >>>> + sg_init_one(&data_sg, buf, buf_size); >>>> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); >>>> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >>>> + cmd.data_sg = &data_sg; >>>> + >>>> + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); >>>> +} >>>> +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); >>>> >>>> list commands are actually for a group, not for the VF. >>>> >>>> The VF was given to let the function gets the PF from it. >>>> >>>> For now, the only existing 'group_type' in the spec is SRIOV, this is why we >>>> hard-coded it internally to match the VF PCI. >>>> >>>> Alternatively, >>>> We can change the API to get the PF and 'group_type' from the caller to better >>>> match future usage. >>>> However, this will require to export the virtio_pci_vf_get_pf_dev() API outside >>>> virtio-pci. >>>> >>>> Do you prefer to change to the latter option ? >>> No, there are several points I wanted to make but this >>> was not one of them. >>> >>> First, for query, I was trying to suggest changing the comment. >>> Something like: >>> + * virtio_pci_admin_list_query - Provides to driver list of commands >>> + * supported for the group including the given member device. >>> + * @dev: member pci device. >> Following your suggestion below, to issue inside virtio the query/use and >> keep its data internally (i.e. on the 'admin_queue' context). >> >> We may suggest the below API for the upper-layers (e.g. vfio) to be >> exported. >> >> bool virtio_pci_admin_supported_cmds(struct pci_dev *pdev, u64 cmds) >> >> It will find the PF from the VF and internally will check on the >> 'admin_queue' context whether the given 'cmds' input is supported. >> >> Its output will be true/false. >> >> Makes sense ? >> >>> >>> >>> >>> Second, I don't think using buf/size like this is necessary. >>> For now we have a small number of commands just work with u64. >> OK, just keep in mind that upon issuing the command towards the controller >> this still needs to be an allocated u64 data on the heap to work properly. >>> >>> Third, while list could be an OK API, the use API does not >>> really work. If you call use with one set of parameters for >>> one VF and another for another then they conflict do they not? >>> >>> So you need virtio core to do the list/use dance for you, >>> save the list of commands on the PF (which again is just u64 for now) >>> and vfio or vdpa or whatnot will just query that. >>> I hope I'm being clear. >> In that case the virtio_pci_admin_list_query() and >> virtio_pci_admin_list_use() won't be exported any more and will be static in >> virtio-pci. >> >> They will be called internally as part of activating the admin_queue and >> will simply get struct virtio_device* (the PF) instead of struct pci_dev >> *pdev. > > Yes - I think some kind of API will be needed to setup/cleanup. > Then 1st call to setup would do the list/use dance? some ref counting? OK, we may work to come in V2 with that option in place. Please note that the initialization 'list/use' commands would be done as part of the admin queue activation but we can't enable the admin queue for the upper layers before that it was done. So, we may consider skipping the ref count set/get as part of the initialization flow with some flag/detection of the list/use commands as the ref count setting enables the admin queue for upper-layers which we would like to prevent by that time. > > And maybe the API should just be > bool virtio_pci_admin_has_legacy_io() This can work as well. In that case, the API will just get the VF PCI to get from it the PF + 'admin_queue' context and will check internally that all current 5 legacy commands are supported. Yishai > > > >>> >>>> >>>> + >>>> +/* >>>> + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device >>>> + * @dev: VF pci_dev >>>> + * @opcode: op code of the io write command >>>> >>>> opcode is actually either VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE >>>> or VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE correct? >>>> >>>> So please just add 2 APIs for this so users don't need to care. >>>> Could be wrappers around these two things. >>>> >>>> >>>> OK. >>>> >>>> We'll export the below 2 APIs [1] which internally will call >>>> virtio_pci_admin_legacy_io_write() with the proper op code hard-coded. >>>> >>>> [1]virtio_pci_admin_legacy_device_io_write() >>>> virtio_pci_admin_legacy_common_io_write() >>>> >>>> Yishai >>>> >>> Makes sense. >> OK, we may do the same split for the 'legacy_io_read' commands to be >> symmetric with the 'legacy_io_write', right ? >> >> Yishai > makes sense. > >>>> + * @offset: starting byte offset within the registers to write to >>>> + * @size: size of the data to write >>>> + * @buf: buffer which holds the data >>>> + * >>>> + * Returns 0 on success, or negative on failure. >>>> + */ >>>> +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, >>>> + u8 offset, u8 size, u8 *buf) >>>> +{ >>>> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >>>> + struct virtio_admin_cmd_legacy_wr_data *data; >>>> + struct virtio_admin_cmd cmd = {}; >>>> + struct scatterlist data_sg; >>>> + int vf_id; >>>> + int ret; >>>> + >>>> + if (!virtio_dev) >>>> + return -ENODEV; >>>> + >>>> + vf_id = pci_iov_vf_id(pdev); >>>> + if (vf_id < 0) >>>> + return vf_id; >>>> + >>>> + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); >>>> + if (!data) >>>> + return -ENOMEM; >>>> + >>>> + data->offset = offset; >>>> + memcpy(data->registers, buf, size); >>>> + sg_init_one(&data_sg, data, sizeof(*data) + size); >>>> + cmd.opcode = cpu_to_le16(opcode); >>>> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >>>> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >>>> + cmd.data_sg = &data_sg; >>>> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >>>> + >>>> + kfree(data); >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); >>>> + >>>> +/* >>>> + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device >>>> + * @dev: VF pci_dev >>>> + * @opcode: op code of the io read command >>>> + * @offset: starting byte offset within the registers to read from >>>> + * @size: size of the data to be read >>>> + * @buf: buffer to hold the returned data >>>> + * >>>> + * Returns 0 on success, or negative on failure. >>>> + */ >>>> +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, >>>> + u8 offset, u8 size, u8 *buf) >>>> +{ >>>> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >>>> + struct virtio_admin_cmd_legacy_rd_data *data; >>>> + struct scatterlist data_sg, result_sg; >>>> + struct virtio_admin_cmd cmd = {}; >>>> + int vf_id; >>>> + int ret; >>>> + >>>> + if (!virtio_dev) >>>> + return -ENODEV; >>>> + >>>> + vf_id = pci_iov_vf_id(pdev); >>>> + if (vf_id < 0) >>>> + return vf_id; >>>> + >>>> + data = kzalloc(sizeof(*data), GFP_KERNEL); >>>> + if (!data) >>>> + return -ENOMEM; >>>> + >>>> + data->offset = offset; >>>> + sg_init_one(&data_sg, data, sizeof(*data)); >>>> + sg_init_one(&result_sg, buf, size); >>>> + cmd.opcode = cpu_to_le16(opcode); >>>> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >>>> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >>>> + cmd.data_sg = &data_sg; >>>> + cmd.result_sg = &result_sg; >>>> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >>>> + >>>> + kfree(data); >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); >>>> + >>>> +/* >>>> + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification >>>> + * information for legacy interface >>>> + * @dev: VF pci_dev >>>> + * @req_bar_flags: requested bar flags >>>> + * @bar: on output the BAR number of the member device >>>> + * @bar_offset: on output the offset within bar >>>> + * >>>> + * Returns 0 on success, or negative on failure. >>>> + */ >>>> +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, >>>> + u8 req_bar_flags, u8 *bar, >>>> + u64 *bar_offset) >>>> +{ >>>> + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); >>>> + struct virtio_admin_cmd_notify_info_result *result; >>>> + struct virtio_admin_cmd cmd = {}; >>>> + struct scatterlist result_sg; >>>> + int vf_id; >>>> + int ret; >>>> + >>>> + if (!virtio_dev) >>>> + return -ENODEV; >>>> + >>>> + vf_id = pci_iov_vf_id(pdev); >>>> + if (vf_id < 0) >>>> + return vf_id; >>>> + >>>> + result = kzalloc(sizeof(*result), GFP_KERNEL); >>>> + if (!result) >>>> + return -ENOMEM; >>>> + >>>> + sg_init_one(&result_sg, result, sizeof(*result)); >>>> + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); >>>> + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); >>>> + cmd.group_member_id = cpu_to_le64(vf_id + 1); >>>> + cmd.result_sg = &result_sg; >>>> + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); >>>> + if (!ret) { >>>> + struct virtio_admin_cmd_notify_info_data *entry; >>>> + int i; >>>> + >>>> + ret = -ENOENT; >>>> + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { >>>> + entry = &result->entries[i]; >>>> + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) >>>> + break; >>>> + if (entry->flags != req_bar_flags) >>>> + continue; >>>> + *bar = entry->bar; >>>> + *bar_offset = le64_to_cpu(entry->offset); >>>> + ret = 0; >>>> + break; >>>> + } >>>> + } >>>> + >>>> + kfree(result); >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); >>>> + >>>> static const struct virtio_config_ops virtio_pci_config_nodev_ops = { >>>> .get = NULL, >>>> .set = NULL, >>>> diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h >>>> new file mode 100644 >>>> index 000000000000..cb916a4bc1b1 >>>> --- /dev/null >>>> +++ b/include/linux/virtio_pci_admin.h >>>> @@ -0,0 +1,18 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H >>>> +#define _LINUX_VIRTIO_PCI_ADMIN_H >>>> + >>>> +#include <linux/types.h> >>>> +#include <linux/pci.h> >>>> + >>>> +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); >>>> +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); >>>> +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, >>>> + u8 offset, u8 size, u8 *buf); >>>> +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, >>>> + u8 offset, u8 size, u8 *buf); >>>> +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, >>>> + u8 req_bar_flags, u8 *bar, >>>> + u64 *bar_offset); >>>> + >>>> +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */ >>>> -- >>>> 2.27.0 >>>> >>>>
On Wed, Oct 25, 2023 at 05:03:55PM +0300, Yishai Hadas wrote: > > Yes - I think some kind of API will be needed to setup/cleanup. > > Then 1st call to setup would do the list/use dance? some ref counting? > > OK, we may work to come in V2 with that option in place. > > Please note that the initialization 'list/use' commands would be done as > part of the admin queue activation but we can't enable the admin queue for > the upper layers before that it was done. I don't know what does this mean. > So, we may consider skipping the ref count set/get as part of the > initialization flow with some flag/detection of the list/use commands as the > ref count setting enables the admin queue for upper-layers which we would > like to prevent by that time. You can init on 1st use but you can't destroy after last use. For symmetry it's better to just have explicit constructor/destructor. > > > > And maybe the API should just be > > bool virtio_pci_admin_has_legacy_io() > > This can work as well. > > In that case, the API will just get the VF PCI to get from it the PF + > 'admin_queue' context and will check internally that all current 5 legacy > commands are supported. > > Yishai Yes, makes sense.
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 6b4766d5abe6..212d68401d2c 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -645,6 +645,17 @@ static struct pci_driver virtio_pci_driver = { .sriov_configure = virtio_pci_sriov_configure, }; +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev) +{ + struct virtio_pci_device *pf_vp_dev; + + pf_vp_dev = pci_iov_get_pf_drvdata(pdev, &virtio_pci_driver); + if (IS_ERR(pf_vp_dev)) + return NULL; + + return &pf_vp_dev->vdev; +} + module_pci_driver(virtio_pci_driver); MODULE_AUTHOR("Anthony Liguori <aliguori@us.ibm.com>"); diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h index a21b9ba01a60..2785e61ed668 100644 --- a/drivers/virtio/virtio_pci_common.h +++ b/drivers/virtio/virtio_pci_common.h @@ -155,4 +155,6 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev) int virtio_pci_modern_probe(struct virtio_pci_device *); void virtio_pci_modern_remove(struct virtio_pci_device *); +struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev); + #endif diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index cc159a8e6c70..00b65e20b2f5 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -719,6 +719,212 @@ static void vp_modern_destroy_avq(struct virtio_device *vdev) vp_dev->del_vq(&vp_dev->admin_vq.info); } +/* + * virtio_pci_admin_list_query - Provides to driver list of commands + * supported for the PCI VF. + * @dev: VF pci_dev + * @buf: buffer to hold the returned list + * @buf_size: size of the given buffer + * + * Returns 0 on success, or negative on failure. + */ +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size) +{ + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); + struct virtio_admin_cmd cmd = {}; + struct scatterlist result_sg; + + if (!virtio_dev) + return -ENODEV; + + sg_init_one(&result_sg, buf, buf_size); + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_QUERY); + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); + cmd.result_sg = &result_sg; + + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); +} +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_query); + +/* + * virtio_pci_admin_list_use - Provides to device list of commands + * used for the PCI VF. + * @dev: VF pci_dev + * @buf: buffer which holds the list + * @buf_size: size of the given buffer + * + * Returns 0 on success, or negative on failure. + */ +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size) +{ + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); + struct virtio_admin_cmd cmd = {}; + struct scatterlist data_sg; + + if (!virtio_dev) + return -ENODEV; + + sg_init_one(&data_sg, buf, buf_size); + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LIST_USE); + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); + cmd.data_sg = &data_sg; + + return vp_modern_admin_cmd_exec(virtio_dev, &cmd); +} +EXPORT_SYMBOL_GPL(virtio_pci_admin_list_use); + +/* + * virtio_pci_admin_legacy_io_write - Write legacy registers of a member device + * @dev: VF pci_dev + * @opcode: op code of the io write command + * @offset: starting byte offset within the registers to write to + * @size: size of the data to write + * @buf: buffer which holds the data + * + * Returns 0 on success, or negative on failure. + */ +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, + u8 offset, u8 size, u8 *buf) +{ + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); + struct virtio_admin_cmd_legacy_wr_data *data; + struct virtio_admin_cmd cmd = {}; + struct scatterlist data_sg; + int vf_id; + int ret; + + if (!virtio_dev) + return -ENODEV; + + vf_id = pci_iov_vf_id(pdev); + if (vf_id < 0) + return vf_id; + + data = kzalloc(sizeof(*data) + size, GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->offset = offset; + memcpy(data->registers, buf, size); + sg_init_one(&data_sg, data, sizeof(*data) + size); + cmd.opcode = cpu_to_le16(opcode); + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); + cmd.group_member_id = cpu_to_le64(vf_id + 1); + cmd.data_sg = &data_sg; + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); + + kfree(data); + return ret; +} +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_write); + +/* + * virtio_pci_admin_legacy_io_read - Read legacy registers of a member device + * @dev: VF pci_dev + * @opcode: op code of the io read command + * @offset: starting byte offset within the registers to read from + * @size: size of the data to be read + * @buf: buffer to hold the returned data + * + * Returns 0 on success, or negative on failure. + */ +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, + u8 offset, u8 size, u8 *buf) +{ + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); + struct virtio_admin_cmd_legacy_rd_data *data; + struct scatterlist data_sg, result_sg; + struct virtio_admin_cmd cmd = {}; + int vf_id; + int ret; + + if (!virtio_dev) + return -ENODEV; + + vf_id = pci_iov_vf_id(pdev); + if (vf_id < 0) + return vf_id; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->offset = offset; + sg_init_one(&data_sg, data, sizeof(*data)); + sg_init_one(&result_sg, buf, size); + cmd.opcode = cpu_to_le16(opcode); + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); + cmd.group_member_id = cpu_to_le64(vf_id + 1); + cmd.data_sg = &data_sg; + cmd.result_sg = &result_sg; + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); + + kfree(data); + return ret; +} +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_read); + +/* + * virtio_pci_admin_legacy_io_notify_info - Read the queue notification + * information for legacy interface + * @dev: VF pci_dev + * @req_bar_flags: requested bar flags + * @bar: on output the BAR number of the member device + * @bar_offset: on output the offset within bar + * + * Returns 0 on success, or negative on failure. + */ +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, + u8 req_bar_flags, u8 *bar, + u64 *bar_offset) +{ + struct virtio_device *virtio_dev = virtio_pci_vf_get_pf_dev(pdev); + struct virtio_admin_cmd_notify_info_result *result; + struct virtio_admin_cmd cmd = {}; + struct scatterlist result_sg; + int vf_id; + int ret; + + if (!virtio_dev) + return -ENODEV; + + vf_id = pci_iov_vf_id(pdev); + if (vf_id < 0) + return vf_id; + + result = kzalloc(sizeof(*result), GFP_KERNEL); + if (!result) + return -ENOMEM; + + sg_init_one(&result_sg, result, sizeof(*result)); + cmd.opcode = cpu_to_le16(VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO); + cmd.group_type = cpu_to_le16(VIRTIO_ADMIN_GROUP_TYPE_SRIOV); + cmd.group_member_id = cpu_to_le64(vf_id + 1); + cmd.result_sg = &result_sg; + ret = vp_modern_admin_cmd_exec(virtio_dev, &cmd); + if (!ret) { + struct virtio_admin_cmd_notify_info_data *entry; + int i; + + ret = -ENOENT; + for (i = 0; i < VIRTIO_ADMIN_CMD_MAX_NOTIFY_INFO; i++) { + entry = &result->entries[i]; + if (entry->flags == VIRTIO_ADMIN_CMD_NOTIFY_INFO_FLAGS_END) + break; + if (entry->flags != req_bar_flags) + continue; + *bar = entry->bar; + *bar_offset = le64_to_cpu(entry->offset); + ret = 0; + break; + } + } + + kfree(result); + return ret; +} +EXPORT_SYMBOL_GPL(virtio_pci_admin_legacy_io_notify_info); + static const struct virtio_config_ops virtio_pci_config_nodev_ops = { .get = NULL, .set = NULL, diff --git a/include/linux/virtio_pci_admin.h b/include/linux/virtio_pci_admin.h new file mode 100644 index 000000000000..cb916a4bc1b1 --- /dev/null +++ b/include/linux/virtio_pci_admin.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_VIRTIO_PCI_ADMIN_H +#define _LINUX_VIRTIO_PCI_ADMIN_H + +#include <linux/types.h> +#include <linux/pci.h> + +int virtio_pci_admin_list_use(struct pci_dev *pdev, u8 *buf, int buf_size); +int virtio_pci_admin_list_query(struct pci_dev *pdev, u8 *buf, int buf_size); +int virtio_pci_admin_legacy_io_write(struct pci_dev *pdev, u16 opcode, + u8 offset, u8 size, u8 *buf); +int virtio_pci_admin_legacy_io_read(struct pci_dev *pdev, u16 opcode, + u8 offset, u8 size, u8 *buf); +int virtio_pci_admin_legacy_io_notify_info(struct pci_dev *pdev, + u8 req_bar_flags, u8 *bar, + u64 *bar_offset); + +#endif /* _LINUX_VIRTIO_PCI_ADMIN_H */
Introduce APIs to execute legacy IO admin commands. It includes: list_query/use, io_legacy_read/write, io_legacy_notify_info. Those APIs will be used by the next patches from this series. Signed-off-by: Yishai Hadas <yishaih@nvidia.com> --- drivers/virtio/virtio_pci_common.c | 11 ++ drivers/virtio/virtio_pci_common.h | 2 + drivers/virtio/virtio_pci_modern.c | 206 +++++++++++++++++++++++++++++ include/linux/virtio_pci_admin.h | 18 +++ 4 files changed, 237 insertions(+) create mode 100644 include/linux/virtio_pci_admin.h