Skip to content

Commit a1069a5

Browse files
junjiemao1lijinxia
junjiemao1
authored and
lijinxia
committed
HV: ioapic: unify the access pattern to RTEs
There are two different ways the current implementation adopts to access ioapic RTEs: 1. As two 32-bit registers (typically named ''low'' and ''high''), or 2. As one 64-bit register (typically named ''rte''). Two issues arise due to the mixed use of these two patterns. 1. Additional conversions are introduced. As an example, ioapic_get_rte() merges two RTE fragments into a uint64_t, while some callers break it back to ''low'' and ''high'' again. 2. It is tricky to choose the proper width of IOAPIC_RTE_xxx constants. SOS boot failure is seen when they are 32-bit due to the following code: /* reg is uint64_t */ vioapic->rtbl[pin].reg &= ~IOAPIC_RTE_REM_IRR; while making them 64-bit leads to implicit narrowing when the RTEs are accessed in the low & high pattern. This patch defines a union ''ioapic_rte'' and unifies the access pattern to IOAPIC and vIOAPIC RTEs. v1 -> v2: * Instead of two 32-bit ''low'' and ''high'', define a union that allows either 32-bit or 64-bit accesses to RTEs. Signed-off-by: Junjie Mao <junjie.mao@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
1 parent 9878543 commit a1069a5

File tree

6 files changed

+209
-213
lines changed

6 files changed

+209
-213
lines changed

‎hypervisor/arch/x86/assign.c

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,12 @@ ptdev_update_irq_handler(struct vm *vm, struct ptdev_remapping_info *entry)
150150
if ((entry->type == PTDEV_INTR_INTX)
151151
&& (entry->ptdev_intr_info.intx.vpin_src
152152
== PTDEV_VPIN_IOAPIC)) {
153-
uint64_t rte;
153+
union ioapic_rte rte;
154154
bool trigger_lvl = false;
155155

156156
/* VPIN_IOAPIC src means we have vioapic enabled */
157157
vioapic_get_rte(vm, entry->ptdev_intr_info.intx.virt_pin, &rte);
158-
if ((rte & IOAPIC_RTE_TRGRMOD) == IOAPIC_RTE_TRGRLVL) {
158+
if ((rte.full & IOAPIC_RTE_TRGRMOD) == IOAPIC_RTE_TRGRLVL) {
159159
trigger_lvl = true;
160160
}
161161

@@ -228,66 +228,65 @@ static void ptdev_build_physical_msi(struct vm *vm, struct ptdev_msi_info *info,
228228
info->pmsi_addr, info->pmsi_data);
229229
}
230230

231-
static uint64_t ptdev_build_physical_rte(struct vm *vm,
231+
static union ioapic_rte
232+
ptdev_build_physical_rte(struct vm *vm,
232233
struct ptdev_remapping_info *entry)
233234
{
234-
uint64_t rte;
235+
union ioapic_rte rte;
235236
uint32_t phys_irq = dev_to_irq(entry->node);
236237
uint32_t vector = dev_to_vector(entry->node);
237238

238239
if (entry->ptdev_intr_info.intx.vpin_src == PTDEV_VPIN_IOAPIC) {
239-
uint64_t vdmask, pdmask;
240-
uint32_t dest, low, high, delmode;
240+
uint64_t vdmask, pdmask, delmode;
241+
uint32_t dest;
242+
union ioapic_rte virt_rte;
241243
bool phys;
242244

243-
vioapic_get_rte(vm, entry->ptdev_intr_info.intx.virt_pin, &rte);
244-
low = rte;
245-
high = rte >> 32;
245+
vioapic_get_rte(vm, entry->ptdev_intr_info.intx.virt_pin,
246+
&virt_rte);
247+
rte = virt_rte;
246248

247249
/* physical destination cpu mask */
248-
phys = ((low & IOAPIC_RTE_DESTMOD) == IOAPIC_RTE_DESTPHY);
249-
dest = high >> APIC_ID_SHIFT;
250+
phys = ((virt_rte.full & IOAPIC_RTE_DESTMOD) == IOAPIC_RTE_DESTPHY);
251+
dest = (uint32_t)(virt_rte.full >> IOAPIC_RTE_DEST_SHIFT);
250252
calcvdest(vm, &vdmask, dest, phys);
251253
pdmask = vcpumask2pcpumask(vm, vdmask);
252254

253255
/* physical delivery mode */
254-
delmode = low & IOAPIC_RTE_DELMOD;
256+
delmode = virt_rte.full & IOAPIC_RTE_DELMOD;
255257
if ((delmode != IOAPIC_RTE_DELFIXED) &&
256258
(delmode != IOAPIC_RTE_DELLOPRI)) {
257259
delmode = IOAPIC_RTE_DELLOPRI;
258260
}
259261

260262
/* update physical delivery mode, dest mode(logical) & vector */
261-
low &= ~(IOAPIC_RTE_DESTMOD |
263+
rte.full &= ~(IOAPIC_RTE_DESTMOD |
262264
IOAPIC_RTE_DELMOD | IOAPIC_RTE_INTVEC);
263-
low |= IOAPIC_RTE_DESTLOG | delmode | vector;
265+
rte.full |= IOAPIC_RTE_DESTLOG | delmode | (uint64_t)vector;
264266

265267
/* update physical dest field */
266-
high &= ~IOAPIC_RTE_DEST;
267-
high |= pdmask << 24;
268+
rte.full &= ~IOAPIC_RTE_DEST_MASK;
269+
rte.full |= pdmask << IOAPIC_RTE_DEST_SHIFT;
268270

269271
dev_dbg(ACRN_DBG_IRQ, "IOAPIC RTE = 0x%x:%x(V) -> 0x%x:%x(P)",
270-
rte >> 32, (uint32_t)rte, high, low);
271-
272-
rte = high;
273-
rte = rte << 32 | low;
272+
virt_rte.u.hi_32, virt_rte.u.lo_32,
273+
rte.u.hi_32, rte.u.lo_32);
274274
} else {
275275
enum vpic_trigger trigger;
276-
uint64_t physical_rte;
276+
union ioapic_rte phys_rte;
277277

278278
/* just update trigger mode */
279-
ioapic_get_rte(phys_irq, &physical_rte);
280-
rte = physical_rte;
281-
rte &= ~IOAPIC_RTE_TRGRMOD;
279+
ioapic_get_rte(phys_irq, &phys_rte);
280+
rte.full = phys_rte.full & (~IOAPIC_RTE_TRGRMOD);
282281
vpic_get_irq_trigger(vm,
283282
entry->ptdev_intr_info.intx.virt_pin, &trigger);
284283
if (trigger == LEVEL_TRIGGER) {
285-
rte |= IOAPIC_RTE_TRGRLVL;
284+
rte.full |= IOAPIC_RTE_TRGRLVL;
286285
}
287286

288287
dev_dbg(ACRN_DBG_IRQ, "IOAPIC RTE = 0x%x:%x(P) -> 0x%x:%x(P)",
289-
physical_rte >> 32, (uint32_t)physical_rte,
290-
rte >> 32, (uint32_t)rte);
288+
phys_rte.u.hi_32, phys_rte.u.lo_32,
289+
rte.u.hi_32, rte.u.lo_32);
291290
}
292291

293292
return rte;
@@ -469,12 +468,13 @@ static void ptdev_intr_handle_irq(struct vm *vm,
469468
switch (entry->ptdev_intr_info.intx.vpin_src) {
470469
case PTDEV_VPIN_IOAPIC:
471470
{
472-
uint64_t rte;
471+
union ioapic_rte rte;
473472
bool trigger_lvl = false;
474473

475474
/* VPIN_IOAPIC src means we have vioapic enabled */
476-
vioapic_get_rte(vm, entry->ptdev_intr_info.intx.virt_pin, &rte);
477-
if ((rte & IOAPIC_RTE_TRGRMOD) == IOAPIC_RTE_TRGRLVL) {
475+
vioapic_get_rte(vm, entry->ptdev_intr_info.intx.virt_pin,
476+
&rte);
477+
if ((rte.full & IOAPIC_RTE_TRGRMOD) == IOAPIC_RTE_TRGRLVL) {
478478
trigger_lvl = true;
479479
}
480480

@@ -487,9 +487,10 @@ static void ptdev_intr_handle_irq(struct vm *vm,
487487
}
488488

489489
dev_dbg(ACRN_DBG_PTIRQ,
490-
"dev-assign: irq=0x%x assert vr: 0x%x vRTE=0x%x",
490+
"dev-assign: irq=0x%x assert vr: 0x%x vRTE=0x%lx",
491491
dev_to_irq(entry->node),
492-
irq_to_vector(dev_to_irq(entry->node)), rte);
492+
irq_to_vector(dev_to_irq(entry->node)),
493+
rte.full);
493494
break;
494495
}
495496
case PTDEV_VPIN_PIC:
@@ -669,14 +670,10 @@ static bool vpin_masked(struct vm *vm, uint8_t virt_pin,
669670
enum ptdev_vpin_source vpin_src)
670671
{
671672
if (vpin_src == PTDEV_VPIN_IOAPIC) {
672-
uint64_t rte;
673+
union ioapic_rte rte;
673674

674675
vioapic_get_rte(vm, virt_pin, &rte);
675-
if ((rte & IOAPIC_RTE_INTMASK) == IOAPIC_RTE_INTMSET) {
676-
return true;
677-
} else {
678-
return false;
679-
}
676+
return ((rte.full & IOAPIC_RTE_INTMASK) == IOAPIC_RTE_INTMSET);
680677
} else {
681678
return vpic_is_pin_mask(vm->vpic, virt_pin);
682679
}
@@ -685,7 +682,7 @@ static bool vpin_masked(struct vm *vm, uint8_t virt_pin,
685682
static void activate_physical_ioapic(struct vm *vm,
686683
struct ptdev_remapping_info *entry)
687684
{
688-
uint64_t rte;
685+
union ioapic_rte rte;
689686
uint32_t phys_irq = dev_to_irq(entry->node);
690687

691688
/* disable interrupt */
@@ -695,7 +692,8 @@ static void activate_physical_ioapic(struct vm *vm,
695692
rte = ptdev_build_physical_rte(vm, entry);
696693

697694
/* set rte entry */
698-
GSI_SET_RTE(phys_irq, rte | IOAPIC_RTE_INTMSET);
695+
rte.full |= IOAPIC_RTE_INTMSET;
696+
ioapic_set_rte(phys_irq, rte);
699697

700698
/* update irq handler according to info in guest */
701699
ptdev_update_irq_handler(vm, entry);
@@ -710,7 +708,7 @@ static void activate_physical_ioapic(struct vm *vm,
710708
int ptdev_intx_pin_remap(struct vm *vm, struct ptdev_intx_info *info)
711709
{
712710
struct ptdev_remapping_info *entry;
713-
uint64_t rte;
711+
union ioapic_rte rte;
714712
uint32_t phys_irq;
715713
uint8_t phys_pin;
716714
bool lowpri = !is_vm0(vm);
@@ -824,7 +822,7 @@ int ptdev_intx_pin_remap(struct vm *vm, struct ptdev_intx_info *info)
824822
&& (entry->ptdev_intr_info.intx.vpin_src
825823
== PTDEV_VPIN_IOAPIC)) {
826824
vioapic_get_rte(vm, entry->ptdev_intr_info.intx.virt_pin, &rte);
827-
if (((uint32_t)rte) == 0x10000) {
825+
if (rte.u.lo_32 == 0x10000U) {
828826
/* disable interrupt */
829827
GSI_MASK_IRQ(phys_irq);
830828
ptdev_deactivate_entry(entry);
@@ -957,7 +955,7 @@ static void get_entry_info(struct ptdev_remapping_info *entry, char *type,
957955
} else {
958956
uint32_t phys_irq = pin_to_irq(
959957
entry->ptdev_intr_info.intx.phys_pin);
960-
uint64_t rte = 0;
958+
union ioapic_rte rte;
961959

962960
if (entry->ptdev_intr_info.intx.vpin_src
963961
== PTDEV_VPIN_IOAPIC) {
@@ -966,8 +964,8 @@ static void get_entry_info(struct ptdev_remapping_info *entry, char *type,
966964
(void)strcpy_s(type, 16, "PIC");
967965
}
968966
ioapic_get_rte(phys_irq, &rte);
969-
*dest = ((rte >> 32) & IOAPIC_RTE_DEST) >> 24;
970-
if ((rte & IOAPIC_RTE_TRGRLVL) != 0U) {
967+
*dest = rte.full >> IOAPIC_RTE_DEST_SHIFT;
968+
if ((rte.full & IOAPIC_RTE_TRGRLVL) != 0UL) {
971969
*lvl_tm = true;
972970
} else {
973971
*lvl_tm = false;

0 commit comments

Comments
 (0)