Skip to content

Commit 3df3c9f

Browse files
shiqingglijinxia
authored andcommitted
hv: vuart: fix 'Shifting value too far'
MISRA-C requires that shift operation cannot exceed the word length. What this patch does: - Fix the bug in 'vuart_init' The register 'dll' and 'dlh' should be uint8_t rather than char. 'dll' is the lower 8-bit of divisor. 'dlh' is the higher 8-bit of divisor. So, the shift value should be 8U rather than 16U. - Fix other data type issues regarding to the registers in 'struct vuart' The registers should be unsigned variables. v1 -> v2: * Use a local variable 'uint8_t value_u8 = (uint8_t)value' to avoid mutiple times type conversion * Use '(uint8_t)divisor' rather than '(uint8_t)(divisor & 0xFFU)' to get the lower 8 bit of 'divisor' Direct type conversion is safe and simple per Xiangyang's suggestion. Tracked-On: #861 Signed-off-by: Shiqing Gao <shiqing.gao@intel.com> Reviewed-by: Junjie Mao <junjie.mao@intel.com>
1 parent de487ff commit 3df3c9f

File tree

2 files changed

+31
-31
lines changed

2 files changed

+31
-31
lines changed

hypervisor/debug/vuart.c

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -133,59 +133,60 @@ static void vuart_toggle_intr(struct vuart *vu)
133133
}
134134
}
135135

136-
static void vuart_write(__unused struct vm_io_handler *hdlr,
137-
struct vm *vm, uint16_t offset_arg,
138-
__unused size_t width, uint32_t value)
136+
static void vuart_write(__unused struct vm_io_handler *hdlr, struct vm *vm,
137+
uint16_t offset_arg, __unused size_t width, uint32_t value)
139138
{
140139
uint16_t offset = offset_arg;
141140
struct vuart *vu = vm_vuart(vm);
141+
uint8_t value_u8 = (uint8_t)value;
142+
142143
offset -= vu->base;
143144
vuart_lock(vu);
144145
/*
145146
* Take care of the special case DLAB accesses first
146147
*/
147148
if ((vu->lcr & LCR_DLAB) != 0) {
148149
if (offset == UART16550_DLL) {
149-
vu->dll = value;
150+
vu->dll = value_u8;
150151
goto done;
151152
}
152153

153154
if (offset == UART16550_DLM) {
154-
vu->dlh = value;
155+
vu->dlh = value_u8;
155156
goto done;
156157
}
157158
}
158159

159160
switch (offset) {
160161
case UART16550_THR:
161-
fifo_putchar(&vu->txfifo, value);
162+
fifo_putchar(&vu->txfifo, value_u8);
162163
vu->thre_int_pending = true;
163164
break;
164165
case UART16550_IER:
165166
/*
166167
* Apply mask so that bits 4-7 are 0
167168
* Also enables bits 0-3 only if they're 1
168169
*/
169-
vu->ier = value & 0x0FU;
170+
vu->ier = value_u8 & 0x0FU;
170171
break;
171172
case UART16550_FCR:
172173
/*
173174
* The FCR_ENABLE bit must be '1' for the programming
174175
* of other FCR bits to be effective.
175176
*/
176-
if ((value & FCR_FIFOE) == 0U) {
177-
vu->fcr = 0;
177+
if ((value_u8 & FCR_FIFOE) == 0U) {
178+
vu->fcr = 0U;
178179
} else {
179-
if ((value & FCR_RFR) != 0U) {
180+
if ((value_u8 & FCR_RFR) != 0U) {
180181
fifo_reset(&vu->rxfifo);
181182
}
182183

183-
vu->fcr = value &
184+
vu->fcr = value_u8 &
184185
(FCR_FIFOE | FCR_DMA | FCR_RX_MASK);
185186
}
186187
break;
187188
case UART16550_LCR:
188-
vu->lcr = value;
189+
vu->lcr = value_u8;
189190
break;
190191
case UART16550_MCR:
191192
/* ignore modem */
@@ -202,7 +203,7 @@ static void vuart_write(__unused struct vm_io_handler *hdlr,
202203
*/
203204
break;
204205
case UART16550_SCR:
205-
vu->scr = value;
206+
vu->scr = value_u8;
206207
break;
207208
default:
208209
/*
@@ -218,14 +219,13 @@ static void vuart_write(__unused struct vm_io_handler *hdlr,
218219
vuart_unlock(vu);
219220
}
220221

221-
static uint32_t vuart_read(__unused struct vm_io_handler *hdlr,
222-
struct vm *vm, uint16_t offset_arg,
223-
__unused size_t width)
222+
static uint32_t vuart_read(__unused struct vm_io_handler *hdlr, struct vm *vm,
223+
uint16_t offset_arg, __unused size_t width)
224224
{
225225
uint16_t offset = offset_arg;
226-
char iir, reg;
227-
uint8_t intr_reason;
226+
uint8_t iir, reg, intr_reason;
228227
struct vuart *vu = vm_vuart(vm);
228+
229229
offset -= vu->base;
230230
vuart_lock(vu);
231231
/*
@@ -295,7 +295,7 @@ static uint32_t vuart_read(__unused struct vm_io_handler *hdlr,
295295
done:
296296
vuart_toggle_intr(vu);
297297
vuart_unlock(vu);
298-
return reg;
298+
return (uint32_t)reg;
299299
}
300300

301301
static void vuart_register_io_handler(struct vm *vm)
@@ -370,8 +370,8 @@ void *vuart_init(struct vm *vm)
370370

371371
/* Set baud rate*/
372372
divisor = UART_CLOCK_RATE / BAUD_9600 / 16U;
373-
vu->dll = divisor;
374-
vu->dlh = divisor >> 16U;
373+
vu->dll = (uint8_t)divisor;
374+
vu->dlh = (uint8_t)(divisor >> 8U);
375375

376376
vu->active = false;
377377
vu->base = COM1_BASE;

hypervisor/include/debug/vuart.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,16 @@ struct fifo {
3939
};
4040

4141
struct vuart {
42-
char data; /* Data register (R/W) */
43-
char ier; /* Interrupt enable register (R/W) */
44-
char lcr; /* Line control register (R/W) */
45-
char mcr; /* Modem control register (R/W) */
46-
char lsr; /* Line status register (R/W) */
47-
char msr; /* Modem status register (R/W) */
48-
char fcr; /* FIFO control register (W) */
49-
char scr; /* Scratch register (R/W) */
50-
char dll; /* Baudrate divisor latch LSB */
51-
char dlh; /* Baudrate divisor latch MSB */
42+
uint8_t data; /* Data register (R/W) */
43+
uint8_t ier; /* Interrupt enable register (R/W) */
44+
uint8_t lcr; /* Line control register (R/W) */
45+
uint8_t mcr; /* Modem control register (R/W) */
46+
uint8_t lsr; /* Line status register (R/W) */
47+
uint8_t msr; /* Modem status register (R/W) */
48+
uint8_t fcr; /* FIFO control register (W) */
49+
uint8_t scr; /* Scratch register (R/W) */
50+
uint8_t dll; /* Baudrate divisor latch LSB */
51+
uint8_t dlh; /* Baudrate divisor latch MSB */
5252

5353
struct fifo rxfifo;
5454
struct fifo txfifo;

0 commit comments

Comments
 (0)