There're some instructions which not support bit 0(w bit) flag but which
memory opcode size is fixed and the memory opcode size is not equal to the
register opcode size. In our code, there is movzx (which opcode is 0F B7)
which memory opcode size is fixed to 16 bits. So add a flag VIE_OP_F_WORD_OP
to indicate a instruction which memory opcode size is fixed to 16 bits.
Tracked-On: #1337
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Reviewed-by: Yin Fengwei <fengwei.yin@intel.com>
1) For some instructions (like movsx, movzx which we support), there're two operands
and the source operand size is not equal to the dest operand size. In this case,
if we update the memory operand size according to the bit 0(w bit) of opcode,
we will lost the register operand size. This patch tries to fix this by calculating
memory operand size when we want to use it.
2) Calculate memory operand size form operand size and the bit 0(w bit) of opcode
when we want to operate on memory operand.
Tracked-On: #1337
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Reviewed-by: Yin Fengwei <fengwei.yin@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
Not every instruction supports the operand-size bit (w). This patch try to correct
what done in commit-id 9df8790 by setting a flag VIE_OP_F_BYTE_OP to indicate which
instruction supports the operand-size bit (w).
This bug is found by removing VMX_PROCBASED_CTLS2_VAPIC_REGS VMCS setting when the
physical doesn't support this APICv feature. However, if emulated this in MRB board,
the android can't boot because when switch to trusty world, it will check
"Delivery Status" in ICR first. It turns out that this Bit Test instruction is not
emulated correctly.
Tracked-On: #1337
Signed-off-by: Qi Yadong <yadong.qi@intel.com>
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Reviewed-by: Yin Fengwei <fengwei.yin@intel.com>
All if . . else if constructs shall be
terminated with an else statement.
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com
move instr_emul_ctxt instance from struct per_cpu_region
to struct vcpu, and rename it from g_inst_ctxt to inst_ctxt
Tracked-On: #1842
Signed-off-by: Jason Chen CJ <jason.cj.chen@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
now the 'cpu_mode' is unused in struct vm_guest_paging,
and there is the same variable in struct acrn_vcpu_arch
Tracked-On: #1842
Signed-off-by: Mingqiang Chi <mingqiang.chi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
separate the private macro/structure/functions, which are just used
in instr_emul.c;
and move instr_emul.h to public include path.
Tracked-On: #1842
Signed-off-by: Minggui Cao <minggui.cao@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
Split decode_prefixes() to 2 small APIs
v1-->v2:
split decode_prefixes to 2 APIs
Tracked-On: #861
Signed-off-by: Mingqiang Chi <mingqiang.chi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
Condtional operators which will add code cyclomatic complexity. So replace the
unnecessary conditional operators.
Tracked-On: #861
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Signed integral type cast to unsigned. : (unsigned long and long)
Tracked-On: #861
Signed-off-by: Zide Chen <zide.chen@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
Misra C requires Function must have only one return entry.
Tracked-On: #861
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
It always return zero, so there is no need to check the return vaule from it.
Just refine it to void.
Tracked-On: #861
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
V1->V2:
rename is_exception_inject to has_exception.
V2->V3:
remove the else statement and add new if statement to check
has_exception.
Misra C requires not allowed to use goto.
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
Misra C requires Function must have only 1 return entry.
Fixed it by use "if ... else ..." format.
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
With SMAP enabled, hypervisor can't access pages that
owned by guest(either SOS or UOS), and an override is
is provided: stac()/clac() to enable/disable access to
guest's memory pages.
Pre-conditon:
Mark hypervisor owned pages as supervisor mode (U/S = 0),
and set all othter memory pages as user mode (U/S = 1).
Tracked-On: #2056
Signed-off-by: Yonghua Huang <yonghua.huang@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
Implicit conversion may result in loss of information or undefined behaviour.
So make it with explicit conversion.
Tracked-On: #861
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
There may the theoretic infinite loop with some code. But actually it doesn't.
This patch make these code more obvious it's not a potentially infinite loop.
Tracked-On: #861
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
This patch does:
- remove the unused API declaration
- fix use of uninitialized variable in instr_emul.c
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
MISRA-C requires that if the return value of a function call is
assigned to a local variable, this value shall be used in all paths.
This patch moves the variable assignment right before the moment that
the variable is used.
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
IEC 61508,ISO 26262 standards highly recommend single-exit rule.
Reduce the count of the "return entries".
Fix the violations which is comply with the cases list below:
1.Function has 2 return entries.
2.The first return entry is used to return the error code of
checking variable whether is valid.
Fix the violations in "if else" format.
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
MISRA-C required every switch statement shall have a default label.
Tracked-On: #861
Signed-off-by: Li, Fei1 <fei1.li@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
MISRA-C requires that the function call in which the returned
value is discarded shall be clearly indicated using (void).
This patch fixes the violations related to the following
function calls.
- instr_check_gva
- vlapic_set_local_intr
- prepare_vm
- enter_s3
- emulate_instruction
- ptdev_intx_pin_remap
- register_mmio_emulation_handler
v1 -> v2:
* discard the return value of enter_s3
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
fix the following integer violations:
1. Signed/unsigned conversion without cast
2. Literal value requires a U suffix
3. Implicit conversion of underlying type
v3 -> v4:
* change the type of npk_loglevel/mem_loglevel/console_loglevel
from uint32_t to uint16_t
v2 -> v3:
* discard the return value of update_ept
* discard changes related to npk loglevel
v1 -> v2:
* remove the unnecessary changes related to the false positive
issues caused by scanning tool
* change the type of the local variable 'vlapic_id' from uint8_t
to uint32_t in function 'vlapic_build_id'
* change the type of the struct member 'flags' in shared_buf from
uint64_t to uint32_t
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
clean up mmio_read/mmio_write in instr_emul.c
- make these two functions inline
- discard the return value and add the pre condition
- rename these two functions to vie_mmio_read/vie_mmio_write
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
- Fix the integer violations related to the following rules:
1. The operands to shift operations (<<, >>) shall be unsigned
integers.
2. The operands to bit operations (&, |, ~) shall be unsigned
integers.
- Replace 12U with CPU_PAGE_SHIFT when it is address shift case.
v1 -> v2:
* use existed MACRO to get bus/slot/func values
* update PCI_SLOT MACRO to make it more straightforward
* remove the incorrect replacement of 12U with CPU_PAGE_SHIFT
dmar_fault_msi_write
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
For data structure types "struct vcpu_arch", its name
shall follow Naming convention.
Naming convention rule:If the data structure type is
used by multi modules, its corresponding logic resource
is exposed to external components (such as SOS, UOS),
and its name meaning is simplistic (such as vcpu, vm),
its name needs prefix "acrn_". Variable name can be
shortened from its data structure type name.
The following udpates are made:
struct vcpu_arch arch_vcpu-->struct acrn_vcpu_arch arch
Tracked-On: #861
Signed-off-by: Xiangyang Wu <xiangyang.wu@linux.intel.com>
For data structure types "struct vcpu", its name is identical
with variable name in the same scope. This is a MISRA C violation.
Naming convention rule:If the data structure type is used by multi
modules, its corresponding logic resource is exposed to external
components (such as SOS, UOS), and its name meaning is simplistic
(such as vcpu, vm), its name needs prefix "acrn_".
The following udpates are made:
struct vcpu *vcpu-->struct acrn_vcpu *vcpu
Tracked-On: #861
Signed-off-by: Xiangyang Wu <xiangyang.wu@linux.intel.com>
The operands to shift operations (<<, >>) shall be unsigned integers.
v1 -> v2:
* replace 12U with CPU_PAGE_SHIFT when it is address shift case.
* replace 6UL with 0x6UL
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
Fix violations whose parameter can be read-only.
This patch only fix the parameter whose name is vcpu.
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
Fix violations for function whose parameter can be read-only.
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
- add missing brackets for 'if/else' statements based on MISRA-C
requirements
v1 -> v2:
* add brackets for each conditions in 'if' statements to improve
the readability
* modify 'ptdev_init' to make the logic clearer
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
1.Function return type inconsistent
2.cast on a constant value
V1->V2 add () to return type
V2->V3 keep the sbuf_get and sbuf_put return code
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
Expression should be boolean immediate before 'if','while' key-words.
V1->V2 add () to bool expression
Tracked-On: #861
Signed-off-by: Huihuang Shi <huihuang.shi@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
struct seg_desc_vmcs is actually for segment descriptor vmcs fields.
Change its name to vmcs_seg_field
Tracked-On: #1231
Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
1. The wrong operand size is assigned in instruction decode phase
if the operand size is 1 byte.
According to the SDM, the bit 0(w bit) of opcode should be checked
first to detect whether the operand size is 1 byte. Then, check
whether there is prefix to overwrite the default operand size.
The original instruction decode doesn't care about the operand
size. But do opsize fixup during instruction emulation phase.
With ACRN we need operand size packed to ioreq and send to DM
after instruction decode.
2. We should always touch the GPA by following opsize to avoid side
effect (especially when GPA is for a MMIO).
Tracked-On: #1337
Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
This patch fixes the following issue pointed by Xiangyang and Junjie.
There are some macro arguments acting as formal parameter names.
Drop such arguments since they make no difference to the expanded
implementation and they might confuse some developers.
Here is an example.
'ptr' is dropped in this patch, which is acting as a formal parameter
name and make no difference to the expanded implementation.
-#define build_atomic_load(name, size, type, ptr) \
+#define build_atomic_load(name, size, type) \
static inline type name(const volatile type *ptr) \
{ \
type ret; \
asm volatile("mov" size " %1,%0" \
: "=r" (ret) \
: "m" (*ptr) \
: "cc", "memory"); \
return ret; \
}
Some minor coding style fixes are also included in this patch.
- use TAB for the alignment rather than mixing TAB with space
- fix some typo in the comments
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
MISRAC checks whether a variable is assigned a value not used in
all branches of a program. Var value which is unused on all paths
can be removed with a consequent improvement in the readability
and efficiency of the code. This patch is used to fix these
violations.
Tracked-On: #861
Signed-off-by: Junjun Shan <junjun.shan@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
MISRAC has requirement about implict conversion: actual to formal
param. This patch is used to fix part of these violations.
1.Add a new structure seg_desc_vmcs to hold the VMCS field address of
segment selector to clean up seg_desc structure.
2.Add the definition of maximum MSI entry and the relevant judgement.
3.The violations in shell.c, logmsg.c will be fixed in other series of
patches with modification of function snprintf(), vsnprintf() and other
related usages.
v1->v2:
*Move the definition of struct seg_desc_vmcs from instr_emul.h to
instr_emul.c.
*Modify the formal parameter type in function definition from uint8_t
to char instead of using cast.
*Drop the const declaration for char data in formal parameter.
v2->v3:
*update the data missing conversion.
*change type of internal parameter len to avoid casting in npklog.c.
*change the conversion from signed char to unsigned int in
uart16550_getc() to solve sign-extension.
Tracked-On: #861
Signed-off-by: Junjun Shan <junjun.shan@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
The vie->opcode should be updated when decode_two_byte_opcode(),
otherwise for two bytes opcode emulate(movzx/movsx) will fail.
Signed-off-by: Qi Yadong <yadong.qi@intel.com>
MISRAC has requirements about literal value requires a U suffix and
signed/unsigned conversion with cast. This patch is used to solve
these violations.
v1->v2
*Drop the cast of sz from uint32_t to int32_t, the signed/unsigned
violation of nchars will be solved by other patch together with
printf/sprintf/console/vuart/uart code.
*Delete the unnecessary L suffix of shifting operand.
Tracked-On: #861
Signed-off-by: Junjun Shan <junjun.shan@intel.com>
Reviewed by: Junjie Mao <junjie.mao@intel.com>
The current movs emulation has issues:
1. it use gva to get/put data.
2. it only support src and dst operand are memory which does not
apply to our case (one of them should be mmio and triggers
EPT voilation).
This patch fix the issue by:
1. convert the address from gva to hva before access it.
2. handle mmio emulation.
Also fix the issue introduced by previous instruction reshuffle
patchset:
1. the desc validation should be only applied to none-64bit mode.
2. gva2gpa should be given correct guest virtual address.
Specailly for movs, we cache the dst gpa if the check during
movs decoding success. And use it directly during movs
emulation.
Tracked-On: #1207
Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
Tested-by: Qi Yadong <yadog.qi@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
- convert function like macros to inline functions based on MISRA-C
requirement
- remove some unused and duplicated macros
Tracked-On: #861
Signed-off-by: Shiqing Gao <shiqing.gao@intel.com>
Reviewed-by: Junjie Mao <junjie.mao@intel.com>
We have two same log message in decode_instruction. It's a little bit confusing.
So, this patch refine the log message to make it more explicit.
BTW, we refine one message in create_vcpu.
Tracked-On: #1136
Signed-off-by: Kaige Fu <kaige.fu@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
For the instructions other than MOVS, one operand is register
and another one is memory which trigger EPT voilation. In this
case, there is one possibility that EPT voilation happens before
guest fault:
the fault is triggered by related guest PTE access bit
voilation (like write to a gva with R/W bit cleared in PTE).
So we do this kind of check and inject exception to guest
accordingly during instruction decoding phase.
Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
Acked-by: Anthony Xu <anthony.xu@intel.com>
