Hi Shameer, Qubingbing On 11/23/20 1:51 PM, Shameerali Kolothum Thodi wrote: > Hi Eric, > >> -----Original Message----- >> From: Eric Auger [mailto:eric.auger@xxxxxxxxxx] >> Sent: 16 November 2020 11:00 >> To: eric.auger.pro@xxxxxxxxx; eric.auger@xxxxxxxxxx; >> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >> kvm@xxxxxxxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxxxxxxxx; will@xxxxxxxxxx; >> joro@xxxxxxxxxx; maz@xxxxxxxxxx; robin.murphy@xxxxxxx; >> alex.williamson@xxxxxxxxxx >> Cc: jean-philippe@xxxxxxxxxx; zhangfei.gao@xxxxxxxxxx; >> zhangfei.gao@xxxxxxxxx; vivek.gautam@xxxxxxx; Shameerali Kolothum >> Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; >> jacob.jun.pan@xxxxxxxxxxxxxxx; yi.l.liu@xxxxxxxxx; tn@xxxxxxxxxxxx; >> nicoleotsuka@xxxxxxxxx; yuzenghui <yuzenghui@xxxxxxxxxx> >> Subject: [PATCH v11 08/13] vfio/pci: Add framework for custom interrupt >> indices >> >> Implement IRQ capability chain infrastructure. All interrupt >> indexes beyond VFIO_PCI_NUM_IRQS are handled as extended >> interrupts. They are registered with a specific type/subtype >> and supported flags. >> >> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >> --- >> drivers/vfio/pci/vfio_pci.c | 99 +++++++++++++++++++++++------ >> drivers/vfio/pci/vfio_pci_intrs.c | 62 ++++++++++++++++++ >> drivers/vfio/pci/vfio_pci_private.h | 14 ++++ >> 3 files changed, 157 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c >> index 2a6cc1a87323..93e03a4a5f32 100644 >> --- a/drivers/vfio/pci/vfio_pci.c >> +++ b/drivers/vfio/pci/vfio_pci.c >> @@ -608,6 +608,14 @@ static void vfio_pci_disable(struct vfio_pci_device >> *vdev) >> >> WARN_ON(iommu_unregister_device_fault_handler(&vdev->pdev->dev)); >> >> + for (i = 0; i < vdev->num_ext_irqs; i++) >> + vfio_pci_set_irqs_ioctl(vdev, VFIO_IRQ_SET_DATA_NONE | >> + VFIO_IRQ_SET_ACTION_TRIGGER, >> + VFIO_PCI_NUM_IRQS + i, 0, 0, NULL); >> + vdev->num_ext_irqs = 0; >> + kfree(vdev->ext_irqs); >> + vdev->ext_irqs = NULL; >> + >> /* Device closed, don't need mutex here */ >> list_for_each_entry_safe(ioeventfd, ioeventfd_tmp, >> &vdev->ioeventfds_list, next) { >> @@ -823,6 +831,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device >> *vdev, int irq_type) >> return 1; >> } else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) { >> return 1; >> + } else if (irq_type >= VFIO_PCI_NUM_IRQS && >> + irq_type < VFIO_PCI_NUM_IRQS + vdev->num_ext_irqs) { >> + return 1; >> } >> >> return 0; >> @@ -1008,7 +1019,7 @@ static long vfio_pci_ioctl(void *device_data, >> info.flags |= VFIO_DEVICE_FLAGS_RESET; >> >> info.num_regions = VFIO_PCI_NUM_REGIONS + vdev->num_regions; >> - info.num_irqs = VFIO_PCI_NUM_IRQS; >> + info.num_irqs = VFIO_PCI_NUM_IRQS + vdev->num_ext_irqs; >> >> if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV)) { >> int ret = vfio_pci_info_zdev_add_caps(vdev, &caps); >> @@ -1187,36 +1198,87 @@ static long vfio_pci_ioctl(void *device_data, >> >> } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { >> struct vfio_irq_info info; >> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 }; >> + unsigned long capsz; >> >> minsz = offsetofend(struct vfio_irq_info, count); >> >> + /* For backward compatibility, cannot require this */ >> + capsz = offsetofend(struct vfio_irq_info, cap_offset); >> + >> if (copy_from_user(&info, (void __user *)arg, minsz)) >> return -EFAULT; >> >> - if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS) >> + if (info.argsz < minsz || >> + info.index >= VFIO_PCI_NUM_IRQS + vdev->num_ext_irqs) >> return -EINVAL; >> >> - switch (info.index) { >> - case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX: >> - case VFIO_PCI_REQ_IRQ_INDEX: >> - break; >> - case VFIO_PCI_ERR_IRQ_INDEX: >> - if (pci_is_pcie(vdev->pdev)) >> - break; >> - fallthrough; >> - default: >> - return -EINVAL; >> - } >> + if (info.argsz >= capsz) >> + minsz = capsz; >> >> info.flags = VFIO_IRQ_INFO_EVENTFD; >> >> - info.count = vfio_pci_get_irq_count(vdev, info.index); >> - >> - if (info.index == VFIO_PCI_INTX_IRQ_INDEX) >> + switch (info.index) { >> + case VFIO_PCI_INTX_IRQ_INDEX: >> info.flags |= (VFIO_IRQ_INFO_MASKABLE | >> VFIO_IRQ_INFO_AUTOMASKED); >> - else >> + break; >> + case VFIO_PCI_MSI_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX: >> + case VFIO_PCI_REQ_IRQ_INDEX: >> info.flags |= VFIO_IRQ_INFO_NORESIZE; >> + break; >> + case VFIO_PCI_ERR_IRQ_INDEX: >> + info.flags |= VFIO_IRQ_INFO_NORESIZE; >> + if (!pci_is_pcie(vdev->pdev)) >> + return -EINVAL; >> + break; >> + default: >> + { >> + struct vfio_irq_info_cap_type cap_type = { >> + .header.id = VFIO_IRQ_INFO_CAP_TYPE, >> + .header.version = 1 }; >> + int ret, i; >> + >> + if (info.index >= VFIO_PCI_NUM_IRQS + >> + vdev->num_ext_irqs) >> + return -EINVAL; >> + info.index = array_index_nospec(info.index, >> + VFIO_PCI_NUM_IRQS + >> + vdev->num_ext_irqs); >> + i = info.index - VFIO_PCI_NUM_IRQS; >> + >> + info.flags = vdev->ext_irqs[i].flags; >> + cap_type.type = vdev->ext_irqs[i].type; >> + cap_type.subtype = vdev->ext_irqs[i].subtype; >> + >> + ret = vfio_info_add_capability(&caps, >> + &cap_type.header, >> + sizeof(cap_type)); >> + if (ret) >> + return ret; >> + } >> + } >> + >> + info.count = vfio_pci_get_irq_count(vdev, info.index); >> + >> + if (caps.size) { >> + info.flags |= VFIO_IRQ_INFO_FLAG_CAPS; >> + if (info.argsz < sizeof(info) + caps.size) { >> + info.argsz = sizeof(info) + caps.size; >> + info.cap_offset = 0; >> + } else { >> + vfio_info_cap_shift(&caps, sizeof(info)); >> + if (copy_to_user((void __user *)arg + >> + sizeof(info), caps.buf, >> + caps.size)) { >> + kfree(caps.buf); >> + return -EFAULT; >> + } >> + info.cap_offset = sizeof(info); >> + } >> + >> + kfree(caps.buf); >> + } >> >> return copy_to_user((void __user *)arg, &info, minsz) ? >> -EFAULT : 0; >> @@ -1235,7 +1297,8 @@ static long vfio_pci_ioctl(void *device_data, >> max = vfio_pci_get_irq_count(vdev, hdr.index); >> >> ret = vfio_set_irqs_validate_and_prepare(&hdr, max, >> - VFIO_PCI_NUM_IRQS, &data_size); >> + VFIO_PCI_NUM_IRQS + vdev->num_ext_irqs, >> + &data_size); >> if (ret) >> return ret; >> >> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c >> index 869dce5f134d..1e785a5f5fb2 100644 >> --- a/drivers/vfio/pci/vfio_pci_intrs.c >> +++ b/drivers/vfio/pci/vfio_pci_intrs.c >> @@ -19,6 +19,7 @@ >> #include <linux/vfio.h> >> #include <linux/wait.h> >> #include <linux/slab.h> >> +#include <linux/nospec.h> >> >> #include "vfio_pci_private.h" >> >> @@ -635,6 +636,24 @@ static int vfio_pci_set_req_trigger(struct >> vfio_pci_device *vdev, >> count, flags, data); >> } >> >> +static int vfio_pci_set_ext_irq_trigger(struct vfio_pci_device *vdev, >> + unsigned int index, unsigned int start, >> + unsigned int count, uint32_t flags, >> + void *data) >> +{ >> + int i; >> + >> + if (start != 0 || count > 1) >> + return -EINVAL; > > It looks like we need to add ! vdev->num_ext_irqs check above > as the vdev->ext_irqs is allocated for "nested" case only. > > Thanks to qubingbing for reporting the below crash, > > [ 5034.902114] Unable to handle kernel paging request at virtual address ffffffffffffff98 > [ 5034.927645] Mem abort info: > ... > [ 5035.088409] pc : vfio_pci_set_ctx_trigger_single+0x2c/0x108 [vfio_pci] > [ 5035.102140] lr : vfio_pci_set_ext_irq_trigger+0x84/0xa0 [vfio_pci] > ... > [ 5035.289655] Call trace: > [ 5035.294796] vfio_pci_set_ctx_trigger_single+0x2c/0x108 [vfio_pci] > [ 5035.307795] vfio_pci_set_ext_irq_trigger+0x84/0xa0 [vfio_pci] > [ 5035.320062] vfio_pci_set_irqs_ioctl+0xf0/0x120 [vfio_pci] > [ 5035.331601] vfio_pci_disable+0x60/0x4e0 [vfio_pci] > [ 5035.341859] vfio_pci_release+0x74/0xb0 [vfio_pci] > > This is observed when the Guest is launched without vsmmu and then "poweroff". > > estuary:/home/vsmmu$ ./qemu-system-aarch64_vsmmu5.20 \ >> -machine virt,gic-version=3 \ >> -cpu host \ >> -smp 1 \ >> -m 1024 \ >> -kernel Image_vsmmu \ >> -initrd rootfs_vsmmu.cpio.gz \ >> -device vfio-pci,host=7d:01.0,id=net0 \ >> -net none \ >> -nographic -D -d -enable-kvm \ >> -append "rdinit=init console=ttyAMA0 ealycon=pl0ll,0x90000000" > > estuary:/$ poweroff > > Could you please take a look. Sure I will. Thanks Eric > > Thanks, > Shameer > > >> + index = array_index_nospec(index, >> + VFIO_PCI_NUM_IRQS + vdev->num_ext_irqs); >> + i = index - VFIO_PCI_NUM_IRQS; >> + >> + return vfio_pci_set_ctx_trigger_single(&vdev->ext_irqs[i].trigger, >> + count, flags, data); >> +} >> + >> int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags, >> unsigned index, unsigned start, unsigned count, >> void *data) >> @@ -684,6 +703,13 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_device >> *vdev, uint32_t flags, >> break; >> } >> break; >> + default: >> + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { >> + case VFIO_IRQ_SET_ACTION_TRIGGER: >> + func = vfio_pci_set_ext_irq_trigger; >> + break; >> + } >> + break; >> } >> >> if (!func) >> @@ -691,3 +717,39 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_device >> *vdev, uint32_t flags, >> >> return func(vdev, index, start, count, flags, data); >> } >> + >> +int vfio_pci_get_ext_irq_index(struct vfio_pci_device *vdev, >> + unsigned int type, unsigned int subtype) >> +{ >> + int i; >> + >> + for (i = 0; i < vdev->num_ext_irqs; i++) { >> + if (vdev->ext_irqs[i].type == type && >> + vdev->ext_irqs[i].subtype == subtype) { >> + return i; >> + } >> + } >> + return -EINVAL; >> +} >> + >> +int vfio_pci_register_irq(struct vfio_pci_device *vdev, >> + unsigned int type, unsigned int subtype, >> + u32 flags) >> +{ >> + struct vfio_ext_irq *ext_irqs; >> + >> + ext_irqs = krealloc(vdev->ext_irqs, >> + (vdev->num_ext_irqs + 1) * sizeof(*ext_irqs), >> + GFP_KERNEL); >> + if (!ext_irqs) >> + return -ENOMEM; >> + >> + vdev->ext_irqs = ext_irqs; >> + >> + vdev->ext_irqs[vdev->num_ext_irqs].type = type; >> + vdev->ext_irqs[vdev->num_ext_irqs].subtype = subtype; >> + vdev->ext_irqs[vdev->num_ext_irqs].flags = flags; >> + vdev->ext_irqs[vdev->num_ext_irqs].trigger = NULL; >> + vdev->num_ext_irqs++; >> + return 0; >> +} >> diff --git a/drivers/vfio/pci/vfio_pci_private.h >> b/drivers/vfio/pci/vfio_pci_private.h >> index 1d9b0f648133..e180b5435c8f 100644 >> --- a/drivers/vfio/pci/vfio_pci_private.h >> +++ b/drivers/vfio/pci/vfio_pci_private.h >> @@ -77,6 +77,13 @@ struct vfio_pci_region { >> u32 flags; >> }; >> >> +struct vfio_ext_irq { >> + u32 type; >> + u32 subtype; >> + u32 flags; >> + struct eventfd_ctx *trigger; >> +}; >> + >> struct vfio_pci_dummy_resource { >> struct resource resource; >> int index; >> @@ -111,6 +118,8 @@ struct vfio_pci_device { >> struct vfio_pci_irq_ctx *ctx; >> int num_ctx; >> int irq_type; >> + struct vfio_ext_irq *ext_irqs; >> + int num_ext_irqs; >> int num_regions; >> struct vfio_pci_region *region; >> u8 msi_qmax; >> @@ -154,6 +163,11 @@ struct vfio_pci_device { >> >> extern void vfio_pci_intx_mask(struct vfio_pci_device *vdev); >> extern void vfio_pci_intx_unmask(struct vfio_pci_device *vdev); >> +extern int vfio_pci_register_irq(struct vfio_pci_device *vdev, >> + unsigned int type, unsigned int subtype, >> + u32 flags); >> +extern int vfio_pci_get_ext_irq_index(struct vfio_pci_device *vdev, >> + unsigned int type, unsigned int subtype); >> >> extern int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, >> uint32_t flags, unsigned index, >> -- >> 2.21.3 > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
