Re: [RFC PATCH] hv: vioapic: simple vioapic set rte


Eddie Dong
 

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Li, Fei1
Sent: Tuesday, August 14, 2018 10:28 AM
To: acrn-dev@...
Subject: [acrn-dev] [RFC PATCH] hv: vioapic: simple vioapic set rte

1. do nothing if nothing changed.
2. update ptdev native ioapic rte when (a) something changed and
(b) the old rte interrupt mask is not masked or the new rte interrupt mask is
not masked.

Signed-off-by: Li, Fei1 <fei1.li@...>
---
hypervisor/dm/vioapic.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/hypervisor/dm/vioapic.c b/hypervisor/dm/vioapic.c index
119eabe..7c1d95d 100644
--- a/hypervisor/dm/vioapic.c
+++ b/hypervisor/dm/vioapic.c
@@ -313,12 +313,12 @@ vioapic_indirect_write(struct vioapic *vioapic,
uint32_t addr, uint32_t data)
if ((regnum >= IOAPIC_REDTBL) &&
(regnum < (IOAPIC_REDTBL + (pincount * 2U)))) {
uint32_t addr_offset = regnum - IOAPIC_REDTBL;
- uint32_t rte_offset = addr_offset / 2U;
+ uint32_t rte_offset = addr_offset >> 1U;
pin = rte_offset;

last = vioapic->rtbl[pin];
new = last;
- if ((addr_offset % 2U) != 0U) {
+ if ((addr_offset & 1U) != 0U) {
new.u.hi_32 = data;
} else {
new.u.lo_32 &= RTBL_RO_BITS;
@@ -336,11 +336,13 @@ vioapic_indirect_write(struct vioapic *vioapic,
uint32_t addr, uint32_t data)
}

changed = last.full ^ new.full;
+ if (changed == 0UL) {
+ return;
+ }
/* pin0 from vpic mask/unmask */
if ((pin == 0U) && ((changed & IOAPIC_RTE_INTMASK) != 0UL)) {
/* mask -> umask */
- if (((last.full & IOAPIC_RTE_INTMASK) != 0UL) &&
- ((new.full & IOAPIC_RTE_INTMASK) == 0UL)) {
+ if ((last.full & IOAPIC_RTE_INTMASK) != 0UL) {
if ((vioapic->vm->wire_mode ==
VPIC_WIRE_NULL) ||
(vioapic->vm->wire_mode ==
@@ -354,8 +356,7 @@ vioapic_indirect_write(struct vioapic *vioapic,
uint32_t addr, uint32_t data)
return;
}
/* unmask -> mask */
- } else if (((last.full & IOAPIC_RTE_INTMASK) == 0UL) &&
- ((new.full & IOAPIC_RTE_INTMASK) != 0UL)) {
+ } else {
if (vioapic->vm->wire_mode ==
VPIC_WIRE_IOAPIC) {
vioapic->vm->wire_mode =
@@ -363,9 +364,6 @@ vioapic_indirect_write(struct vioapic *vioapic,
uint32_t addr, uint32_t data)
dev_dbg(ACRN_DBG_IOAPIC,
"vpic wire mode -> INTR");
}
- } else {
- /* Can never happen since IOAPIC_RTE_INTMASK
- * is changed. */
Don't remove this. It is MISRAC requirement.

}
}
vioapic->rtbl[pin] = new;
@@ -405,15 +403,9 @@ vioapic_indirect_write(struct vioapic *vioapic,
uint32_t addr, uint32_t data)
vioapic_send_intr(vioapic, pin);
}

- /* remap for active: interrupt mask -> unmask
- * remap for deactive: interrupt mask & vector set to 0
- * remap for trigger mode change
- * remap for polarity change
- */
- if ( ((changed & IOAPIC_RTE_INTMASK) != 0UL) ||
- ((changed & IOAPIC_RTE_TRGRMOD) != 0UL) ||
- ((changed & IOAPIC_RTE_INTPOL ) != 0UL) ) {
-
+ /* remap for ptdev */
+ if (((new.full & IOAPIC_RTE_INTMASK) == 0UL) ||
+ ((last.full & IOAPIC_RTE_INTMASK) == 0U)) {
0UL not 0U

For this change, can you explain a little?
Why (changed & IOAPIC_RTE_INTMASK) != 0UL) is worse?
And why remove TRIGMOD/INTPOL change condition?

/* VM enable intr */
struct ptdev_intx_info intx;

--
2.7.4


Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.