hv:Cleanup ptdev lock

Move ptdev lock out from add_intx_remapping/add_msix_remapping
/remove_intx_remapping/remove_msix_remapping and make it protect
the whole add entry/remove entry process

v3-->v4:
  --move ptdev lock out

v2-->v3:
  -- still use ptdev lock for add/remove ptdev entry
Tracked-On: #861
Signed-off-by: Mingqiang Chi <mingqiang.chi@intel.com>
Reviewed-by: Jason Chen CJ <jason.cj.chen@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@intel.com>
Reviewed-by: Eddie Dong <eddie.dong@intel.com>
This commit is contained in:
Mingqiang Chi 2018-11-19 11:30:04 +08:00 committed by lijinxia
parent b7bbf81287
commit 297a264a74
2 changed files with 19 additions and 33 deletions

View File

@ -195,12 +195,10 @@ static struct ptdev_remapping_info *add_msix_remapping(struct acrn_vm *vm,
DEFINE_MSI_SID(phys_sid, phys_bdf, entry_nr); DEFINE_MSI_SID(phys_sid, phys_bdf, entry_nr);
DEFINE_MSI_SID(virt_sid, virt_bdf, entry_nr); DEFINE_MSI_SID(virt_sid, virt_bdf, entry_nr);
spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &phys_sid, NULL); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &phys_sid, NULL);
if (entry == NULL) { if (entry == NULL) {
if (ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &virt_sid, vm) != NULL) { if (ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &virt_sid, vm) != NULL) {
pr_err("MSIX re-add vbdf%x", virt_bdf); pr_err("MSIX re-add vbdf%x", virt_bdf);
spinlock_release(&ptdev_lock);
return NULL; return NULL;
} }
@ -233,7 +231,6 @@ static struct ptdev_remapping_info *add_msix_remapping(struct acrn_vm *vm,
* required. */ * required. */
} }
spinlock_release(&ptdev_lock);
dev_dbg(ACRN_DBG_IRQ, "VM%d MSIX add vector mapping vbdf%x:pbdf%x idx=%d", dev_dbg(ACRN_DBG_IRQ, "VM%d MSIX add vector mapping vbdf%x:pbdf%x idx=%d",
vm->vm_id, virt_bdf, phys_bdf, entry_nr); vm->vm_id, virt_bdf, phys_bdf, entry_nr);
@ -247,10 +244,9 @@ remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf, uint32_t entr
struct ptdev_remapping_info *entry; struct ptdev_remapping_info *entry;
DEFINE_MSI_SID(virt_sid, virt_bdf, entry_nr); DEFINE_MSI_SID(virt_sid, virt_bdf, entry_nr);
spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &virt_sid, vm); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &virt_sid, vm);
if (entry == NULL) { if (entry == NULL) {
goto END; return;
} }
if (is_entry_active(entry)) { if (is_entry_active(entry)) {
@ -265,9 +261,6 @@ remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf, uint32_t entr
release_entry(entry); release_entry(entry);
END:
spinlock_release(&ptdev_lock);
} }
/* add intx entry for a vm, based on intx id (phys_pin) /* add intx entry for a vm, based on intx id (phys_pin)
@ -289,12 +282,10 @@ static struct ptdev_remapping_info *add_intx_remapping(struct acrn_vm *vm, uint8
return NULL; return NULL;
} }
spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &phys_sid, NULL); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &phys_sid, NULL);
if (entry == NULL) { if (entry == NULL) {
if (ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm) != NULL) { if (ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm) != NULL) {
pr_err("INTX re-add vpin %d", virt_pin); pr_err("INTX re-add vpin %d", virt_pin);
spinlock_release(&ptdev_lock);
return NULL; return NULL;
} }
entry = alloc_entry(vm, PTDEV_INTR_INTX); entry = alloc_entry(vm, PTDEV_INTR_INTX);
@ -323,7 +314,6 @@ static struct ptdev_remapping_info *add_intx_remapping(struct acrn_vm *vm, uint8
* required. */ * required. */
} }
spinlock_release(&ptdev_lock);
dev_dbg(ACRN_DBG_IRQ, "VM%d INTX add pin mapping vpin%d:ppin%d", vm->vm_id, virt_pin, phys_pin); dev_dbg(ACRN_DBG_IRQ, "VM%d INTX add pin mapping vpin%d:ppin%d", vm->vm_id, virt_pin, phys_pin);
return entry; return entry;
@ -338,10 +328,10 @@ static void remove_intx_remapping(const struct acrn_vm *vm, uint8_t virt_pin, bo
pic_pin ? PTDEV_VPIN_PIC : PTDEV_VPIN_IOAPIC; pic_pin ? PTDEV_VPIN_PIC : PTDEV_VPIN_IOAPIC;
DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src); DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src);
spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm);
if (entry == NULL) { if (entry == NULL) {
goto END; return;
} }
if (is_entry_active(entry)) { if (is_entry_active(entry)) {
@ -360,9 +350,6 @@ static void remove_intx_remapping(const struct acrn_vm *vm, uint8_t virt_pin, bo
} }
release_entry(entry); release_entry(entry);
END:
spinlock_release(&ptdev_lock);
} }
static void ptdev_intr_handle_irq(struct acrn_vm *vm, static void ptdev_intr_handle_irq(struct acrn_vm *vm,
@ -479,9 +466,7 @@ void ptdev_intx_ack(struct acrn_vm *vm, uint8_t virt_pin,
struct ptdev_remapping_info *entry; struct ptdev_remapping_info *entry;
DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src); DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src);
spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm);
spinlock_release(&ptdev_lock);
if (entry == NULL) { if (entry == NULL) {
return; return;
} }
@ -536,7 +521,6 @@ int ptdev_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf,
*/ */
spinlock_obtain(&ptdev_lock); spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &virt_sid, vm); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_MSI, &virt_sid, vm);
spinlock_release(&ptdev_lock);
if (entry == NULL) { if (entry == NULL) {
/* VM0 we add mapping dynamically */ /* VM0 we add mapping dynamically */
if (is_vm0(vm)) { if (is_vm0(vm)) {
@ -544,6 +528,7 @@ int ptdev_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf,
virt_bdf, virt_bdf, entry_nr); virt_bdf, virt_bdf, entry_nr);
if (entry == NULL) { if (entry == NULL) {
pr_err("dev-assign: msi entry exist in others"); pr_err("dev-assign: msi entry exist in others");
spinlock_release(&ptdev_lock);
return -ENODEV; return -ENODEV;
} }
} else { } else {
@ -552,10 +537,13 @@ int ptdev_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf,
* the caller. * the caller.
*/ */
pr_err("dev-assign: msi entry not exist"); pr_err("dev-assign: msi entry not exist");
spinlock_release(&ptdev_lock);
return -ENODEV; return -ENODEV;
} }
} }
spinlock_release(&ptdev_lock);
/* handle destroy case */ /* handle destroy case */
if (is_entry_active(entry) && (info->vmsi_data == 0U)) { if (is_entry_active(entry) && (info->vmsi_data == 0U)) {
info->pmsi_data = 0U; info->pmsi_data = 0U;
@ -630,11 +618,9 @@ int ptdev_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
goto END; goto END;
} }
#endif #endif
/* query if we have virt to phys mapping */ /* query if we have virt to phys mapping */
spinlock_obtain(&ptdev_lock); spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm); entry = ptdev_lookup_entry_by_sid(PTDEV_INTR_INTX, &virt_sid, vm);
spinlock_release(&ptdev_lock);
if (entry == NULL) { if (entry == NULL) {
if (is_vm0(vm)) { if (is_vm0(vm)) {
bool pic_pin = (vpin_src == PTDEV_VPIN_PIC); bool pic_pin = (vpin_src == PTDEV_VPIN_PIC);
@ -651,10 +637,8 @@ int ptdev_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
pic_ioapic_pin_map[virt_pin], pic_ioapic_pin_map[virt_pin],
pic_pin ? PTDEV_VPIN_IOAPIC : pic_pin ? PTDEV_VPIN_IOAPIC :
PTDEV_VPIN_PIC); PTDEV_VPIN_PIC);
spinlock_obtain(&ptdev_lock);
entry = ptdev_lookup_entry_by_sid( entry = ptdev_lookup_entry_by_sid(
PTDEV_INTR_INTX, &tmp_vsid, vm); PTDEV_INTR_INTX, &tmp_vsid, vm);
spinlock_release(&ptdev_lock);
if (entry != NULL) { if (entry != NULL) {
need_switch_vpin_src = true; need_switch_vpin_src = true;
} }
@ -673,6 +657,7 @@ int ptdev_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
if (entry == NULL) { if (entry == NULL) {
pr_err("%s, add intx remapping failed", pr_err("%s, add intx remapping failed",
__func__); __func__);
spinlock_release(&ptdev_lock);
return -ENODEV; return -ENODEV;
} }
} }
@ -681,6 +666,7 @@ int ptdev_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
* everytime a pin get unmask, here filter out pins * everytime a pin get unmask, here filter out pins
* not get mapped. * not get mapped.
*/ */
spinlock_release(&ptdev_lock);
goto END; goto END;
} }
} }
@ -696,6 +682,7 @@ int ptdev_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
virt_pin, entry->vm->vm_id); virt_pin, entry->vm->vm_id);
entry->virt_sid.value = virt_sid.value; entry->virt_sid.value = virt_sid.value;
} }
spinlock_release(&ptdev_lock);
activate_physical_ioapic(vm, entry); activate_physical_ioapic(vm, entry);
dev_dbg(ACRN_DBG_IRQ, dev_dbg(ACRN_DBG_IRQ,
@ -726,7 +713,9 @@ int ptdev_add_intx_remapping(struct acrn_vm *vm, uint8_t virt_pin, uint8_t phys_
return -EINVAL; return -EINVAL;
} }
spinlock_obtain(&ptdev_lock);
entry = add_intx_remapping(vm, virt_pin, phys_pin, pic_pin); entry = add_intx_remapping(vm, virt_pin, phys_pin, pic_pin);
spinlock_release(&ptdev_lock);
return (entry != NULL) ? 0 : -ENODEV; return (entry != NULL) ? 0 : -ENODEV;
} }
@ -736,7 +725,9 @@ int ptdev_add_intx_remapping(struct acrn_vm *vm, uint8_t virt_pin, uint8_t phys_
*/ */
void ptdev_remove_intx_remapping(const struct acrn_vm *vm, uint8_t virt_pin, bool pic_pin) void ptdev_remove_intx_remapping(const struct acrn_vm *vm, uint8_t virt_pin, bool pic_pin)
{ {
spinlock_obtain(&ptdev_lock);
remove_intx_remapping(vm, virt_pin, pic_pin); remove_intx_remapping(vm, virt_pin, pic_pin);
spinlock_release(&ptdev_lock);
} }
/* except vm0, Device Model should call this function to pre-hold ptdev msi /* except vm0, Device Model should call this function to pre-hold ptdev msi
@ -751,7 +742,9 @@ int ptdev_add_msix_remapping(struct acrn_vm *vm, uint16_t virt_bdf,
uint32_t i; uint32_t i;
for (i = 0U; i < vector_count; i++) { for (i = 0U; i < vector_count; i++) {
spinlock_obtain(&ptdev_lock);
entry = add_msix_remapping(vm, virt_bdf, phys_bdf, i); entry = add_msix_remapping(vm, virt_bdf, phys_bdf, i);
spinlock_release(&ptdev_lock);
if (entry == NULL) { if (entry == NULL) {
return -ENODEV; return -ENODEV;
} }
@ -769,7 +762,9 @@ void ptdev_remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf,
uint32_t i; uint32_t i;
for (i = 0U; i < vector_count; i++) { for (i = 0U; i < vector_count; i++) {
spinlock_obtain(&ptdev_lock);
remove_msix_remapping(vm, virt_bdf, i); remove_msix_remapping(vm, virt_bdf, i);
spinlock_release(&ptdev_lock);
} }
} }
@ -848,7 +843,6 @@ void get_ptdev_info(char *str_arg, size_t str_max)
size -= len; size -= len;
str += len; str += len;
spinlock_obtain(&ptdev_lock);
for (idx = 0U; idx < CONFIG_MAX_PT_IRQ_ENTRIES; idx++) { for (idx = 0U; idx < CONFIG_MAX_PT_IRQ_ENTRIES; idx++) {
entry = &ptdev_irq_entries[idx]; entry = &ptdev_irq_entries[idx];
if (is_entry_active(entry)) { if (is_entry_active(entry)) {
@ -858,7 +852,6 @@ void get_ptdev_info(char *str_arg, size_t str_max)
len = snprintf(str, size, "\r\n%d\t%s\t%d\t0x%X\t0x%X", len = snprintf(str, size, "\r\n%d\t%s\t%d\t0x%X\t0x%X",
entry->vm->vm_id, type, irq, vector, dest); entry->vm->vm_id, type, irq, vector, dest);
if (len >= size) { if (len >= size) {
spinlock_release(&ptdev_lock);
goto overflow; goto overflow;
} }
size -= len; size -= len;
@ -871,14 +864,12 @@ void get_ptdev_info(char *str_arg, size_t str_max)
(vbdf & 0xff00U) >> 8U, (vbdf & 0xff00U) >> 8U,
(vbdf & 0xf8U) >> 3U, vbdf & 0x7U); (vbdf & 0xf8U) >> 3U, vbdf & 0x7U);
if (len >= size) { if (len >= size) {
spinlock_release(&ptdev_lock);
goto overflow; goto overflow;
} }
size -= len; size -= len;
str += len; str += len;
} }
} }
spinlock_release(&ptdev_lock);
snprintf(str, size, "\r\n"); snprintf(str, size, "\r\n");
return; return;

View File

@ -11,7 +11,6 @@
#define PTDEV_BITMAP_ARRAY_SIZE INT_DIV_ROUNDUP(CONFIG_MAX_PT_IRQ_ENTRIES, 64U) #define PTDEV_BITMAP_ARRAY_SIZE INT_DIV_ROUNDUP(CONFIG_MAX_PT_IRQ_ENTRIES, 64U)
struct ptdev_remapping_info ptdev_irq_entries[CONFIG_MAX_PT_IRQ_ENTRIES]; struct ptdev_remapping_info ptdev_irq_entries[CONFIG_MAX_PT_IRQ_ENTRIES];
static uint64_t ptdev_entry_bitmaps[PTDEV_BITMAP_ARRAY_SIZE]; static uint64_t ptdev_entry_bitmaps[PTDEV_BITMAP_ARRAY_SIZE];
spinlock_t ptdev_lock; spinlock_t ptdev_lock;
bool is_entry_active(const struct ptdev_remapping_info *entry) bool is_entry_active(const struct ptdev_remapping_info *entry)
@ -86,7 +85,6 @@ ptdev_dequeue_softirq(struct acrn_vm *vm)
return entry; return entry;
} }
/* require ptdev_lock protect */
struct ptdev_remapping_info * struct ptdev_remapping_info *
alloc_entry(struct acrn_vm *vm, uint32_t intr_type) alloc_entry(struct acrn_vm *vm, uint32_t intr_type)
{ {
@ -115,14 +113,13 @@ alloc_entry(struct acrn_vm *vm, uint32_t intr_type)
return entry; return entry;
} }
/* require ptdev_lock protect */
void void
release_entry(struct ptdev_remapping_info *entry) release_entry(struct ptdev_remapping_info *entry)
{ {
uint64_t rflags; uint64_t rflags;
/* /*
* remove entry from softirq list.the ptdev_lock * remove entry from softirq list
* is required before calling release_entry. * is required before calling release_entry.
*/ */
spinlock_irqsave_obtain(&entry->vm->softirq_dev_lock, &rflags); spinlock_irqsave_obtain(&entry->vm->softirq_dev_lock, &rflags);
@ -132,7 +129,6 @@ release_entry(struct ptdev_remapping_info *entry)
bitmap_clear_nolock((entry->ptdev_entry_id) & 0x3FU, &ptdev_entry_bitmaps[(entry->ptdev_entry_id) >> 6U]); bitmap_clear_nolock((entry->ptdev_entry_id) & 0x3FU, &ptdev_entry_bitmaps[(entry->ptdev_entry_id) >> 6U]);
} }
/* require ptdev_lock protect */
static void static void
release_all_entries(const struct acrn_vm *vm) release_all_entries(const struct acrn_vm *vm)
{ {
@ -214,7 +210,6 @@ void ptdev_init(void)
} }
spinlock_init(&ptdev_lock); spinlock_init(&ptdev_lock);
register_softirq(SOFTIRQ_PTDEV, ptdev_softirq); register_softirq(SOFTIRQ_PTDEV, ptdev_softirq);
} }