Date   

[PATCH v2 0/7] instruction decoding refine

Yin, Fengwei <fengwei.yin@...>
 

According to SDM, we should check whether the gva access from
guest is valid. If it's not, correct exception should be injected.

We only need to emulate the instructions which access the
memory and could trigger EPT violation or APIC access VM exit.
It's not necessary to cover the instructions which doesn't access
memory.

To eliminate the side effect of access mmio, we move all gva check
to instruction decoding phase. To make instruction emulation always
sucess.

There are two types of instructions:
- MOVS/STO. The gva is from DI/SI
- Others. The gva is from instruction decoding
We cover both of them.

The TODO work in next cyle refine:
- Fix issue in movs
- Optimize movs
- enable smep/smap check during gva2gpa
- cache the gpa during instruction decoding and avoid gva2gpa
during instruction emulation

ChangeLog:
v1 -> v2:
- Drop the lock prefix check.
- print message if MMIO is access with RIP as indirect access base

Yin Fengwei (7):
hv: extend the decode_modrm
hv: fix use without initialization build error
hv: remove unnecessary check for gva
hv: move check out of vie_calculate_gla
hv: add new function to get gva for MOVS/STO instruction
hv: add failure case check for MOVS/STO
hv: add gva check for the case gva is from instruction decode

hypervisor/arch/x86/guest/instr_emul.c | 451 ++++++++++++++++---------
1 file changed, 290 insertions(+), 161 deletions(-)

--
2.17.0


Re: [PATCH 1/8] hv: add lock prefix check for exception

Yin, Fengwei <fengwei.yin@...>
 

On 8/15/2018 10:27 AM, Xu, Anthony wrote:
Did you see VM_Exit with lock prefix?
If not, can we add error message for now.
I tried android UOS and there is no VM exit with lock prefix. And
I suppose we could drop this patch because lock prefix only could
be used with few instructions. It's very unlikely MMIO operation
has lock prefix.

BTW, there is no message for RIP indirect access to MMIO. But this
depends on the toolchain very much. I would like to keep it.

Regards
Yin, Fengwei

Anthony

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Yin, Fengwei
Sent: Tuesday, August 14, 2018 5:56 PM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH 1/8] hv: add lock prefix check for exception


On 08/15/2018 05:07 AM, Xu, Anthony wrote:
Does this trigger GP in guest before triggering VM_Exit?
According to SDM 25.1.1 Relative priority of Faults and VM Exits,
The faults with higher priority than VM exit doesn't include lock
prefix.

Also there is no other place talking about the lock prefix behavior
in guest. So I assume it triggers VM exit instead of GP.

Regards
Yin, Fengwei


Anthony



-----Original Message-----
From: acrn-dev@... [mailto:acrn-
dev@...]
On Behalf Of Yin, Fengwei
Sent: Tuesday, August 14, 2018 6:04 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH 1/8] hv: add lock prefix check for exception

According to SDM Vol2 - Instruction Set Reference:
Some instructions can't work with prefix lock or can't work in
some situations (destination is not a memory operand).

We add the lock prefix check for the instructions we support and
inject UD if lock shouldn't be used.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 34 ++++++++++++++++++++++----
hypervisor/arch/x86/guest/instr_emul.h | 3 ++-
2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c
b/hypervisor/arch/x86/guest/instr_emul.c
index 521f0d36..222fa033 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -56,20 +56,24 @@
#define VIE_OP_F_MOFFSET (1U << 2) /* 16/32/64-bit immediate
moffset */
#define VIE_OP_F_NO_MODRM (1U << 3)
#define VIE_OP_F_NO_GLA_VERIFICATION (1U << 4)
+#define VIE_OP_F_NO_LOCK (1U << 5) /* UD if lock prefix is used
*/

static const struct instr_emul_vie_op two_byte_opcodes[256] = {
[0xB6] = {
.op_type = VIE_OP_TYPE_MOVZX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xB7] = {
.op_type = VIE_OP_TYPE_MOVZX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xBA] = {
.op_type = VIE_OP_TYPE_BITTEST,
- .op_flags = VIE_OP_F_IMM8,
+ .op_flags = VIE_OP_F_IMM8 | VIE_OP_F_NO_LOCK,
},
[0xBE] = {
.op_type = VIE_OP_TYPE_MOVSX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
};

@@ -79,32 +83,41 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
},
[0x2B] = {
.op_type = VIE_OP_TYPE_SUB,
+ .op_flags = VIE_OP_F_NO_LOCK, /* 0x2B has register as dst */
},
[0x39] = {
.op_type = VIE_OP_TYPE_CMP,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x3B] = {
.op_type = VIE_OP_TYPE_CMP,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x88] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x89] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x8A] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x8B] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xA1] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM,
+ .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM |
+ VIE_OP_F_NO_LOCK,
},
[0xA3] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM,
+ .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM |
+ VIE_OP_F_NO_LOCK,
},
[0xA4] = {
.op_type = VIE_OP_TYPE_MOVS,
@@ -125,14 +138,15 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
[0xC6] = {
/* XXX Group 11 extended opcode - not just MOV */
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_IMM8,
+ .op_flags = VIE_OP_F_IMM8 | VIE_OP_F_NO_LOCK,
},
[0xC7] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_IMM,
+ .op_flags = VIE_OP_F_IMM | VIE_OP_F_NO_LOCK,
},
[0x23] = {
.op_type = VIE_OP_TYPE_AND,
+ .op_flags = VIE_OP_F_NO_LOCK, /* 0x23 has register as dst */
},
[0x80] = {
/* Group 1 extended opcode */
@@ -151,9 +165,11 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
},
[0x84] = {
.op_type = VIE_OP_TYPE_TEST,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x85] = {
.op_type = VIE_OP_TYPE_TEST,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x08] = {
.op_type = VIE_OP_TYPE_OR,
@@ -1714,6 +1730,8 @@ static int decode_prefixes(struct instr_emul_vie
*vie,
vie->repz_present = 1U;
} else if (x == 0xF2U) {
vie->repnz_present = 1U;
+ } else if (x == 0xF0U) {
+ vie->lock_prefix = 1U;
} else if (segment_override(x, &vie->segment_register)) {
vie->seg_override = 1U;
} else {
@@ -2114,6 +2132,12 @@ static int local_decode_instruction(enum
vm_cpu_mode cpu_mode,
return -1;
}

+ /* If lock is used with instruction doesn't allow lock,
+ * inject invalid opcode exception UD to guest.
+ */
+ if (vie->lock_prefix && (vie->op.op_flags & VIE_OP_F_NO_LOCK))
+ return -1;
+
vie->decoded = 1U; /* success */

return 0;
diff --git a/hypervisor/arch/x86/guest/instr_emul.h
b/hypervisor/arch/x86/guest/instr_emul.h
index 6132332e..a7a20bdb 100644
--- a/hypervisor/arch/x86/guest/instr_emul.h
+++ b/hypervisor/arch/x86/guest/instr_emul.h
@@ -103,7 +103,8 @@ struct instr_emul_vie {
repnz_present:1, /* REPNE/REPNZ prefix */
opsize_override:1, /* Operand size override */
addrsize_override:1, /* Address size override */
- seg_override:1; /* Segment override */
+ seg_override:1, /* Segment override */
+ lock_prefix:1; /* has LOCK prefix */

uint8_t mod:2, /* ModRM byte */
reg:4,
--
2.17.0





Re: [PATCH] ipu: virtio-ipu4 as default IPU DM

Eddie Dong
 

Acked-by: Eddie Dong <eddie.dong@...>

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Zhai, Edwin
Sent: Wednesday, August 15, 2018 3:00 PM
To: acrn-dev@...
Cc: Yew, Chang Ching <chang.ching.yew@...>; Bandi, Kushal
<kushal.bandi@...>; Yu, Ong Hock <ong.hock.yu@...>;
Gopalakrishnan, Karthik L <karthik.l.gopalakrishnan@...>; Gao,
Shiqing <shiqing.gao@...>; Zhai, Edwin <edwin.zhai@...>
Subject: Re: [acrn-dev] [PATCH] ipu: virtio-ipu4 as default IPU DM

Reviewed-by: Edwin Zhai <edwin.zhai@...>

On Tue, 14 Aug 2018, Yew, Chang Ching wrote:

Change-Id: I9dd504da79d31863be4c95bcec21fcc9640bd336
Signed-off-by: Yew, Chang Ching <chang.ching.yew@...>
---
devicemodel/samples/apl-mrb/launch_uos.sh | 74
+++++++++++++----------
1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/devicemodel/samples/apl-mrb/launch_uos.sh
b/devicemodel/samples/apl-mrb/launch_uos.sh
index 8cadc40..b977780 100755
--- a/devicemodel/samples/apl-mrb/launch_uos.sh
+++ b/devicemodel/samples/apl-mrb/launch_uos.sh
@@ -1,5 +1,7 @@
#!/bin/bash

+ipu_passthrough=0
+
function launch_clearlinux()
{
if [ ! -f "/data/$5/$5.img" ]; then
@@ -54,22 +56,26 @@ echo "0000:00:0f.0" >
/sys/bus/pci/devices/0000:00:0f.0/driver/unbind
echo "0000:00:0f.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0 -if [ -d
"/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" >
/sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0 -# please use virtual
slot 22 for i2c 0:16.0 to make sure that the i2c controller -# could
get the same virtaul BDF as physical BDF -if [ -d
"/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" >
/sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" >
/sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c
controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" >
/sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

# for sd card passthrough - SDXC/MMC Host Controller 00:1b.0 @@
-210,22 +216,26 @@ echo "0000:00:18.0" >
/sys/bus/pci/devices/0000:00:18.0/driver/unbind
echo "0000:00:18.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0 -if [ -d
"/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" >
/sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0 -# please use virtual
slot 22 for i2c 0:16.0 to make sure that the i2c controller -# could
get the same virtaul BDF as physical BDF -if [ -d
"/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" >
/sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" >
/sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c
controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" >
/sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

#for memsize setting
--
2.18.0




Best Rgds,
Edwin


Re: [PATCH] ipu: virtio-ipu4 as default IPU DM

Eddie Dong
 

Please move forward...
Acked-by: Eddie Dong <eddie.dong@...>

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Minggui Cao
Sent: Wednesday, August 15, 2018 2:02 PM
To: acrn-dev@...
Cc: Yew, Chang Ching <chang.ching.yew@...>; Bandi, Kushal
<kushal.bandi@...>; Yu, Ong Hock <ong.hock.yu@...>;
Gopalakrishnan, Karthik L <karthik.l.gopalakrishnan@...>; Gao,
Shiqing <shiqing.gao@...>
Subject: Re: [acrn-dev] [PATCH] ipu: virtio-ipu4 as default IPU DM

Reviewed-by: Minggui Cao <minggui.cao@...>

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Yew, Chang Ching
Sent: Tuesday, August 14, 2018 4:55 PM
To: acrn-dev@...
Cc: Yew, Chang Ching <chang.ching.yew@...>; Bandi, Kushal
<kushal.bandi@...>; Yu, Ong Hock <ong.hock.yu@...>;
Gopalakrishnan, Karthik L <karthik.l.gopalakrishnan@...>; Gao,
Shiqing <shiqing.gao@...>
Subject: [acrn-dev] [PATCH] ipu: virtio-ipu4 as default IPU DM

Change-Id: I9dd504da79d31863be4c95bcec21fcc9640bd336
Signed-off-by: Yew, Chang Ching <chang.ching.yew@...>
---
devicemodel/samples/apl-mrb/launch_uos.sh | 74 +++++++++++++----------
1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/devicemodel/samples/apl-mrb/launch_uos.sh
b/devicemodel/samples/apl-mrb/launch_uos.sh
index 8cadc40..b977780 100755
--- a/devicemodel/samples/apl-mrb/launch_uos.sh
+++ b/devicemodel/samples/apl-mrb/launch_uos.sh
@@ -1,5 +1,7 @@
#!/bin/bash

+ipu_passthrough=0
+
function launch_clearlinux()
{
if [ ! -f "/data/$5/$5.img" ]; then
@@ -54,22 +56,26 @@ echo "0000:00:0f.0" >
/sys/bus/pci/devices/0000:00:0f.0/driver/unbind
echo "0000:00:0f.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0 -if [ -d
"/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0 -# please use virtual slot 22 for
i2c 0:16.0 to make sure that the i2c controller -# could get the same virtaul
BDF as physical BDF -if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" >
/sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c
controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" >
/sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

# for sd card passthrough - SDXC/MMC Host Controller 00:1b.0 @@
-210,22 +216,26 @@ echo "0000:00:18.0" >
/sys/bus/pci/devices/0000:00:18.0/driver/unbind
echo "0000:00:18.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0 -if [ -d
"/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0 -# please use virtual slot 22 for
i2c 0:16.0 to make sure that the i2c controller -# could get the same virtaul
BDF as physical BDF -if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" >
/sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c
controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" >
/sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

#for memsize setting
--
2.18.0






Re: [PATCH] HV: remove 'spinlock_rfags' declaration

Yonghua Huang
 

Have updated and PRed.

Thanks.
-Yonghua

-----Original Message-----
From: Dong, Eddie
Sent: Wednesday, August 15, 2018 17:01
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: RE: [acrn-dev] [PATCH] HV: remove 'spinlock_rfags' declaration

Acked-by: Eddie Dong <eddie.dong@...>

BTW, it is not necessary to initialize it to 0.

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Yonghua Huang
Sent: Wednesday, August 15, 2018 9:09 PM
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: [acrn-dev] [PATCH] HV: remove 'spinlock_rfags' declaration

- remove the hardcoding loccal variable name 'cpu_int_vale'

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/arch/x86/ioapic.c | 13 ++++---
hypervisor/arch/x86/irq.c | 73
++++++++++++++++++---------------------
hypervisor/common/ptdev.c | 25 +++++++-------
hypervisor/debug/logmsg.c | 23 ++++++------
hypervisor/include/arch/x86/cpu.h | 15 +++-----
hypervisor/include/lib/spinlock.h | 15 ++++----
6 files changed, 74 insertions(+), 90 deletions(-)

diff --git a/hypervisor/arch/x86/ioapic.c
b/hypervisor/arch/x86/ioapic.c index 954905b..20a3ee5 100644
--- a/hypervisor/arch/x86/ioapic.c
+++ b/hypervisor/arch/x86/ioapic.c
@@ -93,17 +93,16 @@ static inline uint32_t ioapic_read_reg32(const
void *ioapic_base, const uint32_t offset) {
uint32_t v;
+ uint64_t rflags = 0;

- spinlock_rflags;
-
- spinlock_irqsave_obtain(&ioapic_lock);
+ spinlock_irqsave_obtain(&ioapic_lock, &rflags);

/* Write IOREGSEL */
mmio_write32(offset, (void *)ioapic_base + IOAPIC_REGSEL);
/* Read IOWIN */
v = mmio_read32((void *)ioapic_base + IOAPIC_WINDOW);

- spinlock_irqrestore_release(&ioapic_lock);
+ spinlock_irqrestore_release(&ioapic_lock, rflags);
return v;
}

@@ -111,16 +110,16 @@ static inline void ioapic_write_reg32(const
void *ioapic_base,
const uint32_t offset, const uint32_t value) {
- spinlock_rflags;
+ uint64_t rflags = 0;

- spinlock_irqsave_obtain(&ioapic_lock);
+ spinlock_irqsave_obtain(&ioapic_lock, &rflags);

/* Write IOREGSEL */
mmio_write32(offset, (void *)ioapic_base + IOAPIC_REGSEL);
/* Write IOWIN */
mmio_write32(value, (void *)ioapic_base + IOAPIC_WINDOW);

- spinlock_irqrestore_release(&ioapic_lock);
+ spinlock_irqrestore_release(&ioapic_lock, rflags);
}

static inline uint64_t
diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c
index
3d9c557..01976b5 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -67,20 +67,19 @@ static uint32_t find_available_vector()
*/
uint32_t irq_mark_used(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return IRQ_INVALID;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
}
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
return irq;
}

@@ -91,19 +90,19 @@ uint32_t irq_mark_used(uint32_t irq) static
uint32_t alloc_irq(void) {
uint32_t i;
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;

for (i = irq_gsi_num(); i < NR_IRQS; i++) {
desc = &irq_desc_array[i];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
break;
}
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}
return (i == NR_IRQS) ? IRQ_INVALID : i; } @@ -121,15 +120,14 @@
static void local_irq_desc_set_vector(uint32_t irq, uint32_t vr)
/* lock version of set vector */
static void irq_desc_set_vector(uint32_t irq, uint32_t vr) {
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
vector_to_irq[vr] = irq;
desc->vector = vr;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}

/* used with holding irq_lock outside */ @@ -173,7 +171,7 @@ int32_t
request_irq(uint32_t irq_arg, {
struct irq_desc *desc;
uint32_t irq = irq_arg, vector;
- spinlock_rflags;
+ uint64_t rflags = 0;

/*
======================================================
* This is low level ISR handler registering function @@ -237,7
+235,7 @@ int32_t request_irq(uint32_t irq_arg,
}

if (desc->action == NULL) {
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->priv_data = priv_data;
desc->action = action_fn;

@@ -246,7 +244,7 @@ int32_t request_irq(uint32_t irq_arg,
*/
(void)strcpy_s(desc->name, 32U, name);

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
} else {
pr_err("%s: request irq(%u) vr(%u) for %s failed,\
already requested", __func__,
@@ -264,9 +262,9 @@ int32_t request_irq(uint32_t irq_arg, uint32_t
irq_desc_alloc_vector(uint32_t irq) {
uint32_t vr = VECTOR_INVALID;
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;

/* irq should be always available at this time */
if (irq >= NR_IRQS) {
@@ -274,7 +272,7 @@ uint32_t irq_desc_alloc_vector(uint32_t irq)
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->vector != VECTOR_INVALID) {
/* already allocated a vector */
goto OUT;
@@ -288,28 +286,27 @@ uint32_t irq_desc_alloc_vector(uint32_t irq)
}
local_irq_desc_set_vector(irq, vr);
OUT:
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
return vr;
}

void irq_desc_try_free_vector(uint32_t irq) {
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
/* legacy irq's vector is reserved and should not be freed */
if ((irq >= NR_IRQS) || (irq < NR_LEGACY_IRQ)) {
return;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->action == NULL) {
_irq_desc_free_vector(irq);
}

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

}

@@ -421,8 +418,8 @@ void partition_mode_dispatch_interrupt(struct
intr_excp_ctx *ctx) int handle_level_interrupt_common(struct irq_desc
*desc,
__unused void *handler_data)
{
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock @@ -434,7
+431,7 @@ int handle_level_interrupt_common(struct irq_desc *desc,
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* mask iopaic pin */
@@ -454,15 +451,15 @@ int handle_level_interrupt_common(struct
irq_desc *desc,
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

return 0;
}

int common_handler_edge(struct irq_desc *desc, __unused void
*handler_data) {
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock @@ -474,7
+471,7 @@ int common_handler_edge(struct irq_desc *desc, __unused
void
*handler_data)
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* Send EOI to LAPIC/IOAPIC IRR */
@@ -485,15 +482,15 @@ int common_handler_edge(struct irq_desc *desc,
__unused void *handler_data)
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

return 0;
}

int common_dev_handler_level(struct irq_desc *desc, __unused void
*handler_data) {
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock @@ -505,7
+502,7 @@ int common_dev_handler_level(struct irq_desc *desc,
__unused
void *handler_data)
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* mask iopaic pin */
@@ -521,7 +518,7 @@ int common_dev_handler_level(struct irq_desc
*desc, __unused void *handler_data)
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

/* we did not unmask irq until guest EOI the vector */
return 0;
@@ -544,26 +541,24 @@ int quick_handler_nolock(struct irq_desc *desc,
__unused void *handler_data)

void update_irq_handler(uint32_t irq, irq_handler_t func) {
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->irq_handler = func;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}

void free_irq(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return;
}
@@ -572,13 +567,13 @@ void free_irq(uint32_t irq)
dev_dbg(ACRN_DBG_IRQ, "[%s] %s irq%d vr:0x%x",
__func__, desc->name, irq, irq_to_vector(irq));

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);

desc->action = NULL;
desc->priv_data = NULL;
memset(desc->name, '\0', 32U);

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
irq_desc_try_free_vector(desc->irq);
}

diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c
index
ef12ce0..19d4800 100644
--- a/hypervisor/common/ptdev.c
+++ b/hypervisor/common/ptdev.c
@@ -30,26 +30,27 @@ static spinlock_t softirq_dev_lock;

static void ptdev_enqueue_softirq(struct ptdev_remapping_info *entry)
{
- spinlock_rflags;
+ uint64_t rflags = 0;
+
/* enqueue request in order, SOFTIRQ_PTDEV will pickup */
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);

/* avoid adding recursively */
list_del(&entry->softirq_node);
/* TODO: assert if entry already in list */
list_add_tail(&entry->softirq_node,
&softirq_dev_entry_list);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
fire_softirq(SOFTIRQ_PTDEV);
}

struct ptdev_remapping_info*
ptdev_dequeue_softirq(void)
{
+ uint64_t rflags = 0;
struct ptdev_remapping_info *entry = NULL;

- spinlock_rflags;
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);

if (!list_empty(&softirq_dev_entry_list)) {
entry = get_first_item(&softirq_dev_entry_list,
@@ -57,7 +58,7 @@ ptdev_dequeue_softirq(void)
list_del_init(&entry->softirq_node);
}

- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
return entry;
}

@@ -86,7 +87,7 @@ alloc_entry(struct vm *vm, enum ptdev_intr_type
type) void release_entry(struct ptdev_remapping_info *entry) {
- spinlock_rflags;
+ uint64_t rflags = 0;

/* remove entry from ptdev_list */
list_del_init(&entry->entry_node);
@@ -95,9 +96,9 @@ release_entry(struct ptdev_remapping_info *entry)
* remove entry from softirq list.the ptdev_lock
* is required before calling release_entry.
*/
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);
list_del_init(&entry->softirq_node);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);

free(entry);
}
@@ -146,7 +147,7 @@ ptdev_activate_entry(struct ptdev_remapping_info
*entry, uint32_t phys_irq) void ptdev_deactivate_entry(struct
ptdev_remapping_info *entry) {
- spinlock_rflags;
+ uint64_t rflags = 0;

atomic_clear32(&entry->active, ACTIVE_FLAG);

@@ -154,9 +155,9 @@ ptdev_deactivate_entry(struct
ptdev_remapping_info
*entry)
entry->allocated_pirq = IRQ_INVALID;

/* remove from softirq list if added */
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);
list_del_init(&entry->softirq_node);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
}

void ptdev_init(void)
diff --git a/hypervisor/debug/logmsg.c b/hypervisor/debug/logmsg.c
index
eb43c5a..b39d0f1 100644
--- a/hypervisor/debug/logmsg.c
+++ b/hypervisor/debug/logmsg.c
@@ -47,13 +47,13 @@ static int do_copy_earlylog(struct shared_buf
*dst_sbuf, {
uint32_t buf_size, valid_size;
uint32_t cur_tail;
- spinlock_rflags;
+ uint64_t rflags = 0;

if ((src_sbuf->ele_size != dst_sbuf->ele_size)
&& (src_sbuf->ele_num != dst_sbuf->ele_num)) {
- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
printf("Error to copy early hvlog: size mismatch\n");
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
return -EINVAL;
}

@@ -87,12 +87,11 @@ void init_logmsg(__unused uint32_t mem_size,
uint32_t flags) void do_logmsg(uint32_t severity, const char *fmt, ...) {
va_list args;
- uint64_t timestamp;
+ uint64_t timestamp, rflags = 0;
uint16_t pcpu_id;
bool do_console_log;
bool do_mem_log;
char *buffer;
- spinlock_rflags;

do_console_log = (((logmsg.flags & LOG_FLAG_STDOUT) != 0U) &&
(severity <= console_loglevel)); @@ -
129,12 +128,12 @@ void
do_logmsg(uint32_t severity, const char *fmt, ...)

/* Check if flags specify to output to stdout */
if (do_console_log) {
- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);

/* Send buffer to stdout */
printf("%s\n\r", buffer);

- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
}

/* Check if flags specify to output to memory */ @@ -169,11 +168,11
@@ void do_logmsg(uint32_t severity, const char *fmt, ...)

void print_logmsg_buffer(uint16_t pcpu_id) {
- spinlock_rflags;
char buffer[LOG_ENTRY_SIZE + 1];
int read_cnt;
struct shared_buf **sbuf;
int is_earlylog = 0;
+ uint64_t rflags = 0;

if (pcpu_id >= phys_cpu_num) {
return;
@@ -187,13 +186,13 @@ void print_logmsg_buffer(uint16_t pcpu_id)
&per_cpu(sbuf, pcpu_id)[ACRN_HVLOG];
}

- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
if ((*sbuf) != NULL) {
printf("CPU%hu: head: 0x%x, tail: 0x%x %s\n\r",
pcpu_id, (*sbuf)->head, (*sbuf)->tail,
(is_earlylog != 0) ? "[earlylog]" : "");
}
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);

do {
uint32_t idx;
@@ -212,8 +211,8 @@ void print_logmsg_buffer(uint16_t pcpu_id)
idx = (read_cnt < LOG_ENTRY_SIZE) ? read_cnt :
LOG_ENTRY_SIZE;
buffer[idx] = '\0';

- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
printf("%s\n\r", buffer);
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
} while (read_cnt > 0);
}
diff --git a/hypervisor/include/arch/x86/cpu.h
b/hypervisor/include/arch/x86/cpu.h
index db928ce..b047aa4 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -418,13 +418,6 @@ void stop_cpus(void);
*timestamp_ptr = ((uint64_t)tsh << 32) | tsl; \
}

-/* Define variable(s) required to save / restore architecture interrupt state.
- * These variable(s) are used in conjunction with the
ESAL_AR_INT_ALL_DISABLE()
- * and ESAL_AR_INT_ALL_RESTORE() macros to hold any data that must be
preserved
- * in order to allow these macros to function correctly.
- */
-#define CPU_INT_CONTROL_VARS uint64_t
cpu_int_value
-
/* Macro to save rflags register */
#define CPU_RFLAGS_SAVE(rflags_ptr) \
{ \
@@ -450,9 +443,9 @@ void stop_cpus(void);
* defined below and CPU_INT_CONTROL_VARS defined above.
*/

-#define CPU_INT_ALL_DISABLE() \
+#define CPU_INT_ALL_DISABLE(p_rflags) \
{ \
- CPU_RFLAGS_SAVE(&cpu_int_value); \
+ CPU_RFLAGS_SAVE(p_rflags); \
CPU_IRQ_DISABLE(); \
}

@@ -463,9 +456,9 @@ void stop_cpus(void);
* NOTE: This macro is used in conjunction with CPU_INT_ALL_DISABLE
* and CPU_INT_CONTROL_VARS defined above.
*/
-#define CPU_INT_ALL_RESTORE() \
+#define CPU_INT_ALL_RESTORE(rflags) \
{ \
- CPU_RFLAGS_RESTORE(cpu_int_value); \
+ CPU_RFLAGS_RESTORE(rflags); \
}

/*
diff --git a/hypervisor/include/lib/spinlock.h
b/hypervisor/include/lib/spinlock.h
index 613e4b0..d6383bc 100644
--- a/hypervisor/include/lib/spinlock.h
+++ b/hypervisor/include/lib/spinlock.h
@@ -63,18 +63,15 @@ static inline void spinlock_release(spinlock_t
*lock)

#endif /* ASSEMBLER */

-#define spinlock_rflags unsigned long cpu_int_value
-
-#define spinlock_irqsave_obtain(l) \
+#define spinlock_irqsave_obtain(lock, p_rflags) \
do { \
- CPU_INT_ALL_DISABLE(); \
- spinlock_obtain(l); \
+ CPU_INT_ALL_DISABLE(p_rflags); \
+ spinlock_obtain(lock); \
} while (0)

-#define spinlock_irqrestore_release(l) \
+#define spinlock_irqrestore_release(lock, rflags) \
do { \
- spinlock_release(l); \
- CPU_INT_ALL_RESTORE(); \
+ spinlock_release(lock); \
+ CPU_INT_ALL_RESTORE(rflags); \
} while (0)
-
#endif /* SPINLOCK_H */
--
2.7.4



Re: [PATCH] hv: Add vrtc emulation support for ACRN partition mode

Eddie Dong
 

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Grandhi, Sainath
Sent: Wednesday, August 15, 2018 1:39 PM
To: acrn-dev@...
Cc: Grandhi, Sainath <sainath.grandhi@...>
Subject: [acrn-dev] [PATCH] hv: Add vrtc emulation support for ACRN
partition mode

This patch adds code to support read-only RTC support for guests run by
partition mode ACRN. It supports RW for CMOS address port 0x70 and RO
for CMOS data port 0x71. Reads to CMOS RAM offsets are fetched by
reading CMOS h/w directly and writes to CMOS offsets are discarded.

Signed-off-by: Sainath Grandhi <sainath.grandhi@...>
---
hypervisor/Makefile | 1 +
hypervisor/arch/x86/guest/vm.c | 3 +-
hypervisor/dm/vrtc.c | 109
+++++++++++++++++++++++++++++++++
hypervisor/include/arch/x86/guest/vm.h | 4 ++
4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644
hypervisor/dm/vrtc.c

diff --git a/hypervisor/Makefile b/hypervisor/Makefile index
d3e14f76..e3022132 100644
--- a/hypervisor/Makefile
+++ b/hypervisor/Makefile
@@ -177,6 +177,7 @@ C_SRCS += dm/vioapic.c ifeq
($(CONFIG_PARTITION_MODE),y) C_SRCS += $(wildcard dm/vpci/*.c)
C_SRCS += $(wildcard partition/*.c)
+C_SRCS += dm/vrtc.c
endif

C_SRCS += bsp/$(CONFIG_PLATFORM)/vm_description.c
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index f2efbf9e..5c7c6e16 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -205,7 +205,7 @@ int create_vm(struct vm_description *vm_desc,
struct vm **rtn_vm)
if (vm_desc->vm_vuart) {
vm->vuart = vuart_init(vm);
}
-
+ vm->vrtc = vrtc_init(vm);
vpci_init(vm);
#endif

@@ -317,6 +317,7 @@ int shutdown_vm(struct vm *vm)

#ifdef CONFIG_PARTITION_MODE
vpci_cleanup(vm);
+ vrtc_deinit(vm);
#endif

free(vm->hw.vcpu_array);
diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c new file mode
100644 index 00000000..bd3a8ef2
--- /dev/null
+++ b/hypervisor/dm/vrtc.c
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2018 Intel Corporation. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause */
+
+#include <hypervisor.h>
+#include <hv_lib.h>
+#include <acrn_common.h>
+#include <hv_arch.h>
+#include <hv_debug.h>
+
+#define CMOS_ADDR_PORT 0x70U
+#define CMOS_DATA_PORT 0x71U
+
+#define RTC_STATUSA 0x0AU /* status register A */
+#define RTCSA_TUP 0x80U /* time update, don't look now */
+
+struct vrtc {
+ struct vm *vm;
By converting pointer to data instance (in next), probably we don't need this one.

+ uint8_t addr; /* RTC register to read or write */ };
+
+static spinlock_t cmos_lock = { .head = 0U, .tail = 0U };
+
+static uint8_t cmos_read(uint8_t addr)
+{
+ pio_write8(addr, CMOS_ADDR_PORT);
+ return pio_read8(CMOS_DATA_PORT);
+}
+
+static bool cmos_update_in_progress(void) {
+ return (cmos_read(RTC_STATUSA) & RTCSA_TUP)?1:0; }
+
+uint8_t cmos_get_reg_val(uint8_t addr)
+{
+ uint8_t reg;
+ int tries = 2000U;
+
+ spinlock_obtain(&cmos_lock);
+
+ /* Make sure an update isn't in progress */
+ while (cmos_update_in_progress() && tries--)
+ ;
+
+ reg = cmos_read(addr);
+
+ spinlock_release(&cmos_lock);
+ return reg;
+}
+
+static uint32_t vrtc_read(__unused struct vm_io_handler *hdlr, struct vm
*vm,
+ uint16_t addr, __unused size_t width)
+{
+ struct vrtc *vrtc = vm->vrtc;
+ uint8_t reg;
+ uint8_t offset;
+
+ offset = vrtc->addr;
+
+ if (addr == CMOS_ADDR_PORT) {
+ return vrtc->addr;
+ }
+
+ reg = cmos_get_reg_val(offset);
+ return reg;
+}
+
+static void vrtc_write(__unused struct vm_io_handler *hdlr, struct vm *vm,
uint16_t addr,
+ size_t width, uint32_t value)
+{
+ struct vrtc *vrtc = vm->vrtc;
+
+
+ if (width != 1U)
+ return;
+
+ if (addr == CMOS_ADDR_PORT) {
+ vrtc->addr = value & 0x7FU;
+ }
+ return;
+}
+
+void *vrtc_init(struct vm *vm)
+{
+ struct vrtc *vrtc;
+ struct vm_io_range range = {
+ .flags = IO_ATTR_RW, .base = CMOS_ADDR_PORT, .len = 2U};
+
+ vrtc = calloc(1U, sizeof(struct vrtc));
+ ASSERT(vrtc != NULL, "");
+
+ vrtc->vm = vm;
+ register_io_emulation_handler(vm, &range, vrtc_read, vrtc_write);
+
+ return vrtc;
+}
+
+void vrtc_deinit(struct vm *vm)
+{
+ if (vm->vrtc == NULL) {
+ return;
+ }
+
+ free(vm->vrtc);
+ vm->vrtc = NULL;
+}
diff --git a/hypervisor/include/arch/x86/guest/vm.h
b/hypervisor/include/arch/x86/guest/vm.h
index 800ab4df..9f5c8a11 100644
--- a/hypervisor/include/arch/x86/guest/vm.h
+++ b/hypervisor/include/arch/x86/guest/vm.h
@@ -166,6 +166,7 @@ struct vm {
#ifdef CONFIG_PARTITION_MODE
struct vm_description *vm_desc;
struct vpci vpci;
+ void *vrtc;
I prefer to use data instance, rather than pointer.
We want to eliminate dynamic memory allocation eventually.

#endif
};

@@ -275,5 +276,8 @@ struct pcpu_vm_desc_mapping {
bool is_bsp;
};
extern const struct pcpu_vm_desc_mapping pcpu_vm_desc_map[];
+
+void *vrtc_init(struct vm *vm);
+void vrtc_deinit(struct vm *vm);
#endif
#endif /* VM_H_ */
--
2.14.1



Re: [PATCH] HV: remove 'spinlock_rfags' declaration

Eddie Dong
 

Acked-by: Eddie Dong <eddie.dong@...>

BTW, it is not necessary to initialize it to 0.

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Yonghua Huang
Sent: Wednesday, August 15, 2018 9:09 PM
To: acrn-dev@...
Cc: Huang, Yonghua <yonghua.huang@...>
Subject: [acrn-dev] [PATCH] HV: remove 'spinlock_rfags' declaration

- remove the hardcoding loccal variable name 'cpu_int_vale'

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/arch/x86/ioapic.c | 13 ++++---
hypervisor/arch/x86/irq.c | 73
++++++++++++++++++---------------------
hypervisor/common/ptdev.c | 25 +++++++-------
hypervisor/debug/logmsg.c | 23 ++++++------
hypervisor/include/arch/x86/cpu.h | 15 +++-----
hypervisor/include/lib/spinlock.h | 15 ++++----
6 files changed, 74 insertions(+), 90 deletions(-)

diff --git a/hypervisor/arch/x86/ioapic.c b/hypervisor/arch/x86/ioapic.c
index 954905b..20a3ee5 100644
--- a/hypervisor/arch/x86/ioapic.c
+++ b/hypervisor/arch/x86/ioapic.c
@@ -93,17 +93,16 @@ static inline uint32_t ioapic_read_reg32(const void
*ioapic_base, const uint32_t offset) {
uint32_t v;
+ uint64_t rflags = 0;

- spinlock_rflags;
-
- spinlock_irqsave_obtain(&ioapic_lock);
+ spinlock_irqsave_obtain(&ioapic_lock, &rflags);

/* Write IOREGSEL */
mmio_write32(offset, (void *)ioapic_base + IOAPIC_REGSEL);
/* Read IOWIN */
v = mmio_read32((void *)ioapic_base + IOAPIC_WINDOW);

- spinlock_irqrestore_release(&ioapic_lock);
+ spinlock_irqrestore_release(&ioapic_lock, rflags);
return v;
}

@@ -111,16 +110,16 @@ static inline void ioapic_write_reg32(const void
*ioapic_base,
const uint32_t offset, const uint32_t value) {
- spinlock_rflags;
+ uint64_t rflags = 0;

- spinlock_irqsave_obtain(&ioapic_lock);
+ spinlock_irqsave_obtain(&ioapic_lock, &rflags);

/* Write IOREGSEL */
mmio_write32(offset, (void *)ioapic_base + IOAPIC_REGSEL);
/* Write IOWIN */
mmio_write32(value, (void *)ioapic_base + IOAPIC_WINDOW);

- spinlock_irqrestore_release(&ioapic_lock);
+ spinlock_irqrestore_release(&ioapic_lock, rflags);
}

static inline uint64_t
diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c index
3d9c557..01976b5 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -67,20 +67,19 @@ static uint32_t find_available_vector()
*/
uint32_t irq_mark_used(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return IRQ_INVALID;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
}
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
return irq;
}

@@ -91,19 +90,19 @@ uint32_t irq_mark_used(uint32_t irq) static
uint32_t alloc_irq(void) {
uint32_t i;
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;

for (i = irq_gsi_num(); i < NR_IRQS; i++) {
desc = &irq_desc_array[i];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
break;
}
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}
return (i == NR_IRQS) ? IRQ_INVALID : i; } @@ -121,15 +120,14 @@
static void local_irq_desc_set_vector(uint32_t irq, uint32_t vr)
/* lock version of set vector */
static void irq_desc_set_vector(uint32_t irq, uint32_t vr) {
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
vector_to_irq[vr] = irq;
desc->vector = vr;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}

/* used with holding irq_lock outside */ @@ -173,7 +171,7 @@ int32_t
request_irq(uint32_t irq_arg, {
struct irq_desc *desc;
uint32_t irq = irq_arg, vector;
- spinlock_rflags;
+ uint64_t rflags = 0;

/* ======================================================
* This is low level ISR handler registering function @@ -237,7 +235,7
@@ int32_t request_irq(uint32_t irq_arg,
}

if (desc->action == NULL) {
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->priv_data = priv_data;
desc->action = action_fn;

@@ -246,7 +244,7 @@ int32_t request_irq(uint32_t irq_arg,
*/
(void)strcpy_s(desc->name, 32U, name);

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
} else {
pr_err("%s: request irq(%u) vr(%u) for %s failed,\
already requested", __func__,
@@ -264,9 +262,9 @@ int32_t request_irq(uint32_t irq_arg, uint32_t
irq_desc_alloc_vector(uint32_t irq) {
uint32_t vr = VECTOR_INVALID;
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;

/* irq should be always available at this time */
if (irq >= NR_IRQS) {
@@ -274,7 +272,7 @@ uint32_t irq_desc_alloc_vector(uint32_t irq)
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->vector != VECTOR_INVALID) {
/* already allocated a vector */
goto OUT;
@@ -288,28 +286,27 @@ uint32_t irq_desc_alloc_vector(uint32_t irq)
}
local_irq_desc_set_vector(irq, vr);
OUT:
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
return vr;
}

void irq_desc_try_free_vector(uint32_t irq) {
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
/* legacy irq's vector is reserved and should not be freed */
if ((irq >= NR_IRQS) || (irq < NR_LEGACY_IRQ)) {
return;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->action == NULL) {
_irq_desc_free_vector(irq);
}

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

}

@@ -421,8 +418,8 @@ void partition_mode_dispatch_interrupt(struct
intr_excp_ctx *ctx) int handle_level_interrupt_common(struct irq_desc
*desc,
__unused void *handler_data)
{
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock @@ -434,7
+431,7 @@ int handle_level_interrupt_common(struct irq_desc *desc,
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* mask iopaic pin */
@@ -454,15 +451,15 @@ int handle_level_interrupt_common(struct
irq_desc *desc,
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

return 0;
}

int common_handler_edge(struct irq_desc *desc, __unused void
*handler_data) {
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock @@ -474,7
+471,7 @@ int common_handler_edge(struct irq_desc *desc, __unused void
*handler_data)
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* Send EOI to LAPIC/IOAPIC IRR */
@@ -485,15 +482,15 @@ int common_handler_edge(struct irq_desc *desc,
__unused void *handler_data)
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

return 0;
}

int common_dev_handler_level(struct irq_desc *desc, __unused void
*handler_data) {
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock @@ -505,7
+502,7 @@ int common_dev_handler_level(struct irq_desc *desc, __unused
void *handler_data)
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* mask iopaic pin */
@@ -521,7 +518,7 @@ int common_dev_handler_level(struct irq_desc
*desc, __unused void *handler_data)
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

/* we did not unmask irq until guest EOI the vector */
return 0;
@@ -544,26 +541,24 @@ int quick_handler_nolock(struct irq_desc *desc,
__unused void *handler_data)

void update_irq_handler(uint32_t irq, irq_handler_t func) {
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->irq_handler = func;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}

void free_irq(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return;
}
@@ -572,13 +567,13 @@ void free_irq(uint32_t irq)
dev_dbg(ACRN_DBG_IRQ, "[%s] %s irq%d vr:0x%x",
__func__, desc->name, irq, irq_to_vector(irq));

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);

desc->action = NULL;
desc->priv_data = NULL;
memset(desc->name, '\0', 32U);

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
irq_desc_try_free_vector(desc->irq);
}

diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c index
ef12ce0..19d4800 100644
--- a/hypervisor/common/ptdev.c
+++ b/hypervisor/common/ptdev.c
@@ -30,26 +30,27 @@ static spinlock_t softirq_dev_lock;

static void ptdev_enqueue_softirq(struct ptdev_remapping_info *entry) {
- spinlock_rflags;
+ uint64_t rflags = 0;
+
/* enqueue request in order, SOFTIRQ_PTDEV will pickup */
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);

/* avoid adding recursively */
list_del(&entry->softirq_node);
/* TODO: assert if entry already in list */
list_add_tail(&entry->softirq_node,
&softirq_dev_entry_list);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
fire_softirq(SOFTIRQ_PTDEV);
}

struct ptdev_remapping_info*
ptdev_dequeue_softirq(void)
{
+ uint64_t rflags = 0;
struct ptdev_remapping_info *entry = NULL;

- spinlock_rflags;
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);

if (!list_empty(&softirq_dev_entry_list)) {
entry = get_first_item(&softirq_dev_entry_list,
@@ -57,7 +58,7 @@ ptdev_dequeue_softirq(void)
list_del_init(&entry->softirq_node);
}

- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
return entry;
}

@@ -86,7 +87,7 @@ alloc_entry(struct vm *vm, enum ptdev_intr_type type)
void release_entry(struct ptdev_remapping_info *entry) {
- spinlock_rflags;
+ uint64_t rflags = 0;

/* remove entry from ptdev_list */
list_del_init(&entry->entry_node);
@@ -95,9 +96,9 @@ release_entry(struct ptdev_remapping_info *entry)
* remove entry from softirq list.the ptdev_lock
* is required before calling release_entry.
*/
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);
list_del_init(&entry->softirq_node);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);

free(entry);
}
@@ -146,7 +147,7 @@ ptdev_activate_entry(struct ptdev_remapping_info
*entry, uint32_t phys_irq) void ptdev_deactivate_entry(struct
ptdev_remapping_info *entry) {
- spinlock_rflags;
+ uint64_t rflags = 0;

atomic_clear32(&entry->active, ACTIVE_FLAG);

@@ -154,9 +155,9 @@ ptdev_deactivate_entry(struct
ptdev_remapping_info *entry)
entry->allocated_pirq = IRQ_INVALID;

/* remove from softirq list if added */
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);
list_del_init(&entry->softirq_node);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
}

void ptdev_init(void)
diff --git a/hypervisor/debug/logmsg.c b/hypervisor/debug/logmsg.c index
eb43c5a..b39d0f1 100644
--- a/hypervisor/debug/logmsg.c
+++ b/hypervisor/debug/logmsg.c
@@ -47,13 +47,13 @@ static int do_copy_earlylog(struct shared_buf
*dst_sbuf, {
uint32_t buf_size, valid_size;
uint32_t cur_tail;
- spinlock_rflags;
+ uint64_t rflags = 0;

if ((src_sbuf->ele_size != dst_sbuf->ele_size)
&& (src_sbuf->ele_num != dst_sbuf->ele_num)) {
- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
printf("Error to copy early hvlog: size mismatch\n");
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
return -EINVAL;
}

@@ -87,12 +87,11 @@ void init_logmsg(__unused uint32_t mem_size,
uint32_t flags) void do_logmsg(uint32_t severity, const char *fmt, ...) {
va_list args;
- uint64_t timestamp;
+ uint64_t timestamp, rflags = 0;
uint16_t pcpu_id;
bool do_console_log;
bool do_mem_log;
char *buffer;
- spinlock_rflags;

do_console_log = (((logmsg.flags & LOG_FLAG_STDOUT) != 0U) &&
(severity <= console_loglevel));
@@ -129,12 +128,12 @@ void do_logmsg(uint32_t severity, const char
*fmt, ...)

/* Check if flags specify to output to stdout */
if (do_console_log) {
- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);

/* Send buffer to stdout */
printf("%s\n\r", buffer);

- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
}

/* Check if flags specify to output to memory */ @@ -169,11 +168,11
@@ void do_logmsg(uint32_t severity, const char *fmt, ...)

void print_logmsg_buffer(uint16_t pcpu_id) {
- spinlock_rflags;
char buffer[LOG_ENTRY_SIZE + 1];
int read_cnt;
struct shared_buf **sbuf;
int is_earlylog = 0;
+ uint64_t rflags = 0;

if (pcpu_id >= phys_cpu_num) {
return;
@@ -187,13 +186,13 @@ void print_logmsg_buffer(uint16_t pcpu_id)
&per_cpu(sbuf, pcpu_id)[ACRN_HVLOG];
}

- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
if ((*sbuf) != NULL) {
printf("CPU%hu: head: 0x%x, tail: 0x%x %s\n\r",
pcpu_id, (*sbuf)->head, (*sbuf)->tail,
(is_earlylog != 0) ? "[earlylog]" : "");
}
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);

do {
uint32_t idx;
@@ -212,8 +211,8 @@ void print_logmsg_buffer(uint16_t pcpu_id)
idx = (read_cnt < LOG_ENTRY_SIZE) ? read_cnt : LOG_ENTRY_SIZE;
buffer[idx] = '\0';

- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
printf("%s\n\r", buffer);
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
} while (read_cnt > 0);
}
diff --git a/hypervisor/include/arch/x86/cpu.h
b/hypervisor/include/arch/x86/cpu.h
index db928ce..b047aa4 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -418,13 +418,6 @@ void stop_cpus(void);
*timestamp_ptr = ((uint64_t)tsh << 32) | tsl; \
}

-/* Define variable(s) required to save / restore architecture interrupt state.
- * These variable(s) are used in conjunction with the
ESAL_AR_INT_ALL_DISABLE()
- * and ESAL_AR_INT_ALL_RESTORE() macros to hold any data that must be
preserved
- * in order to allow these macros to function correctly.
- */
-#define CPU_INT_CONTROL_VARS uint64_t
cpu_int_value
-
/* Macro to save rflags register */
#define CPU_RFLAGS_SAVE(rflags_ptr) \
{ \
@@ -450,9 +443,9 @@ void stop_cpus(void);
* defined below and CPU_INT_CONTROL_VARS defined above.
*/

-#define CPU_INT_ALL_DISABLE() \
+#define CPU_INT_ALL_DISABLE(p_rflags) \
{ \
- CPU_RFLAGS_SAVE(&cpu_int_value); \
+ CPU_RFLAGS_SAVE(p_rflags); \
CPU_IRQ_DISABLE(); \
}

@@ -463,9 +456,9 @@ void stop_cpus(void);
* NOTE: This macro is used in conjunction with CPU_INT_ALL_DISABLE
* and CPU_INT_CONTROL_VARS defined above.
*/
-#define CPU_INT_ALL_RESTORE() \
+#define CPU_INT_ALL_RESTORE(rflags) \
{ \
- CPU_RFLAGS_RESTORE(cpu_int_value); \
+ CPU_RFLAGS_RESTORE(rflags); \
}

/*
diff --git a/hypervisor/include/lib/spinlock.h
b/hypervisor/include/lib/spinlock.h
index 613e4b0..d6383bc 100644
--- a/hypervisor/include/lib/spinlock.h
+++ b/hypervisor/include/lib/spinlock.h
@@ -63,18 +63,15 @@ static inline void spinlock_release(spinlock_t *lock)

#endif /* ASSEMBLER */

-#define spinlock_rflags unsigned long cpu_int_value
-
-#define spinlock_irqsave_obtain(l) \
+#define spinlock_irqsave_obtain(lock, p_rflags) \
do { \
- CPU_INT_ALL_DISABLE(); \
- spinlock_obtain(l); \
+ CPU_INT_ALL_DISABLE(p_rflags); \
+ spinlock_obtain(lock); \
} while (0)

-#define spinlock_irqrestore_release(l) \
+#define spinlock_irqrestore_release(lock, rflags) \
do { \
- spinlock_release(l); \
- CPU_INT_ALL_RESTORE(); \
+ spinlock_release(lock); \
+ CPU_INT_ALL_RESTORE(rflags); \
} while (0)
-
#endif /* SPINLOCK_H */
--
2.7.4



Re: [PATCH v5 1/5] hv: pirq: refactor irq/vector allocation/free code

Eddie Dong
 

By looking at the data structure:
struct irq_desc {
uint32_t irq; /* index to irq_desc_base */
enum irq_state used; /* this irq have assigned to device */
enum irq_desc_state state; /* irq_desc status */
uint32_t vector; /* assigned vector */

int (*irq_handler)(struct irq_desc *irq_desc, void *handler_data);
/* callback for irq flow handling */
irq_action_t action; /* callback registered from component */
void *priv_data; /* irq_action private data */
char name[32]; /* name of component */

spinlock_t irq_lock;
uint64_t *irq_cnt; /* this irq cnt happened on CPUs */
uint64_t irq_lost_cnt;
};


From revisit p.o.v., I want to ask:
1: Why we need name here? This is device concept which should be removed or was removed.
2: "used" only needs one bit. Do we need a byte? It can be combined with state/
3: Why we need "irq" here? The array index of irq_desc already shows this, right? Correct me if I am wrong.
4: What is the difference between handler and action? Can we combine them?
5: *irq_cnt is stranger... Why?

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Yan, Like
Sent: Wednesday, August 15, 2018 10:45 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v5 1/5] hv: pirq: refactor irq/vector
allocation/free code

This commit refactors the irq_desc and vector allocation/free codes, with:
1) add a global spinlock to protect the irq_desc/vector allocation;

2) three pairs of funtions are defined for irq_desc/vector allocation and
deallocation:
- uint32_t alloc_irq_desc(uint32_t irq)
- if irq is valid, mark the irq_desc as used, or else, alloc an available
irq_desc
- return: irq which is an index to the irq_desc on success, IRQ_INVALID on
failure.

- void free_irq_desc(uint32_t irq)
- mark the irq_desc unused.

- uint32_t alloc_vector_for_irq(uint32_t irq)
- alloc an available vector, and bind it to irq;
- return: valid vector on success, VECTOR_INVALID on failure.

- void free_vector_for_irq(uint32_t irq)
- free vector and clear certain field of irq_desc.

- uint32_t prepare_irq_resource(uint32_t irq)
- alloc irq_desc and vector by calling alloc_irq_desc() and
irq_desc_alloc_vector
according to irq type before setup irq;
- return: irq which is an index to the irq_desc on success, IRQ_INVALID on
failure.
Sounds like you still mixed multiple things in one patch...
It can be further splitted.

- void release_irq_resource(uint32_t irq)
- release dynamical allocated irq_desc and vector.

Signed-off-by: Yan, Like <like.yan@...>
Reviewed-by: Anthony Xu <anthony.xu@...>
---
hypervisor/arch/x86/ioapic.c | 4 +-
hypervisor/arch/x86/irq.c | 306 +++++++++++++++---------------
hypervisor/include/arch/x86/irq.h | 7 +
hypervisor/include/common/irq.h | 7 -
4 files changed, 161 insertions(+), 163 deletions(-)

diff --git a/hypervisor/arch/x86/ioapic.c b/hypervisor/arch/x86/ioapic.c
index 954905b6..c3d7a858 100644
--- a/hypervisor/arch/x86/ioapic.c
+++ b/hypervisor/arch/x86/ioapic.c
@@ -362,7 +362,7 @@ void setup_ioapic_irq(void)
}

/* pinned irq before use it */
- if (irq_mark_used(gsi) >= NR_IRQS) {
+ if (alloc_irq_desc(gsi) >= NR_IRQS) {
pr_err("failed to alloc IRQ[%d]", gsi);
gsi++;
continue;
@@ -372,7 +372,7 @@ void setup_ioapic_irq(void)
* for legacy irq, reserved vector and never free
*/
if (gsi < NR_LEGACY_IRQ) {
- vr = irq_desc_alloc_vector(gsi);
+ vr = alloc_vector_for_irq(gsi);
if (vr > NR_MAX_VECTOR) {
pr_err("failed to alloc VR");
gsi++;
diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c index
3d9c5574..9e20bc06 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -8,6 +8,7 @@
#include <softirq.h>

static spinlock_t exception_spinlock = { .head = 0U, .tail = 0U, };
+static spinlock_t irq_alloc_spinlock = { .head = 0U, .tail = 0U, };

static struct irq_desc irq_desc_array[NR_IRQS]; static uint32_t
vector_to_irq[NR_MAX_VECTOR + 1]; @@ -62,101 +63,198 @@ static
uint32_t find_available_vector() }

/*
- * check and set irq to be assigned
- * return: IRQ_INVALID if irq already assigned otherwise return irq
+ * find an available irq_desc between nr_gsi ~ NR_IRQS-1
*/
-uint32_t irq_mark_used(uint32_t irq)
+static inline uint32_t find_available_irq_desc()
We can call it as find_avaliable_irq_desc, or find_available_irq
Conceptually, we don't want to introduce another concept: irq_desc. (It can stay with an array only, not a new concept).
Even though you are using irq_desc contents to indicate the free of an IRQ#.



{
+ uint32_t i;
struct irq_desc *desc;

- spinlock_rflags;
-
- if (irq >= NR_IRQS) {
- return IRQ_INVALID;
+ for (i = irq_gsi_num(); i < NR_IRQS; i++) {
+ desc = &irq_desc_array[i];
+ if (desc->used == IRQ_NOT_ASSIGNED) {
+ return i;
+ }
}

- desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
- if (desc->used == IRQ_NOT_ASSIGNED) {
- desc->used = IRQ_ASSIGNED;
- }
- spinlock_irqrestore_release(&desc->irq_lock);
- return irq;
+ return IRQ_INVALID;
}

/*
- * system find available irq and set assigned
- * return: irq, VECTOR_INVALID not found
+ * alloc an irq_desc if req_irq is IRQ_INVALID, or else set assigned
+ only
+ * return: valid irq as index to irq_desc on success, IRQ_INVALID on
+ failure
*/
-static uint32_t alloc_irq(void)
+uint32_t alloc_irq_desc(uint32_t req_irq)
Ditto

{
- uint32_t i;
+ uint32_t irq = req_irq;
struct irq_desc *desc;

spinlock_rflags;

- for (i = irq_gsi_num(); i < NR_IRQS; i++) {
- desc = &irq_desc_array[i];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&irq_alloc_spinlock);
+ if (irq == IRQ_INVALID) {
+ irq = find_available_irq_desc();
+ }
+
+ if (irq < NR_IRQS) {
+ desc = &irq_desc_array[irq];
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
- spinlock_irqrestore_release(&desc->irq_lock);
- break;
+ spinlock_irqrestore_release(&irq_alloc_spinlock);
+ return irq;
}
- spinlock_irqrestore_release(&desc->irq_lock);
}
- return (i == NR_IRQS) ? IRQ_INVALID : i;
+
+ spinlock_irqrestore_release(&irq_alloc_spinlock);
+ pr_err("Failed to alloc irq desc, req_irq[%u]", req_irq);
+ return IRQ_INVALID;
}

-/* need irq_lock protection before use */ -static void
local_irq_desc_set_vector(uint32_t irq, uint32_t vr)
+/*
+ * free irq_desc: set irq_desc unassigned */ void
+free_irq_desc(uint32_t irq)
Ditto

{
struct irq_desc *desc;
+ spinlock_rflags;

- desc = &irq_desc_array[irq];
- vector_to_irq[vr] = irq;
- desc->vector = vr;
+ spinlock_irqsave_obtain(&irq_alloc_spinlock);
+
+ if (irq < NR_IRQS) {
+ desc = &irq_desc_array[irq];
+ desc->used = IRQ_NOT_ASSIGNED;
+ desc->state = IRQ_DESC_PENDING;
+ }
+
+ spinlock_irqrestore_release(&irq_alloc_spinlock);
}

-/* lock version of set vector */
-static void irq_desc_set_vector(uint32_t irq, uint32_t vr)
+/*
+ * alloc an vectror and bind it to irq
+ * retval: valid vector num on susccess, VECTOR_INVALID on failure.
+ */
+uint32_t alloc_vector_for_irq(uint32_t irq)
The name is stranger....
Allocate_vector,
Bind_vector_with_irq / bind_vector
...

{
+ uint32_t vr;
struct irq_desc *desc;

spinlock_rflags;

+ if (irq >= NR_IRQS) {
+ pr_err("invalid irq[%u] to alloc vector", irq);
+ return VECTOR_INVALID;
+ }
+
desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
- vector_to_irq[vr] = irq;
- desc->vector = vr;
- spinlock_irqrestore_release(&desc->irq_lock);
+
+ spinlock_irqsave_obtain(&irq_alloc_spinlock);
+ if (desc->vector != VECTOR_INVALID) {
+ pr_err("%s: vector[%u] already allocated for irq[%u]",
+ __func__, desc->vector, irq);
+ vr = VECTOR_INVALID;
+ goto OUT;
+ }
+
+ vr = find_available_vector();
If we go with allocate_vector(irq), will it be simpler?

Or we should directly loop search here. Otherwsie it is confusing.

+
+ /* FLAT mode, an irq connected to the same vector of each cpu */
+ if (vr > VECTOR_DYNAMIC_END) {
+ pr_err("no vector invalid for irq[%u]", irq);
+ vr = VECTOR_INVALID;
+ } else {
+ desc->vector = vr;
+ vector_to_irq[vr] = irq;
+ }
+OUT:
+ spinlock_irqrestore_release(&irq_alloc_spinlock);
+ return vr;
}

-/* used with holding irq_lock outside */ -static void
_irq_desc_free_vector(uint32_t irq)
+/* unbind the vector to irq and free the vector */ void
+free_vector_for_irq(uint32_t irq)
{
struct irq_desc *desc;
uint32_t vr;
- uint16_t pcpu_id;
+ spinlock_rflags;

if (irq >= NR_IRQS) {
return;
}

desc = &irq_desc_array[irq];
-
+ spinlock_irqsave_obtain(&irq_alloc_spinlock);
vr = desc->vector;
- desc->used = IRQ_NOT_ASSIGNED;
- desc->state = IRQ_DESC_PENDING;
desc->vector = VECTOR_INVALID;

vr &= NR_MAX_VECTOR;
if (vector_to_irq[vr] == irq) {
vector_to_irq[vr] = IRQ_INVALID;
}
+ spinlock_irqrestore_release(&irq_alloc_spinlock);
+}

- for (pcpu_id = 0U; pcpu_id < phys_cpu_num; pcpu_id++) {
- per_cpu(irq_count, pcpu_id)[irq] = 0UL;
+
+/*
+ * The function prepares irq and vector before registering handler.
+ * There are four cases:
+ * case 1: req_irq = IRQ_INVALID
+ * caller did not know which irq to use, and want system to
+ * allocate available irq for it. These irq are in range:
+ * nr_gsi ~ NR_IRQS
+ * an irq will be allocated and a vector will be assigned to this
+ * irq automatically.
+ * case 2: req_irq >= NR_LAGACY_IRQ and irq < nr_gsi
+ * caller want to add device ISR handler into ioapic pins.
+ * a vector will automatically assigned.
+ * case 3: req_irq >=0 and req_irq < NR_LAGACY_IRQ
+ * caller want to add device ISR handler into ioapic pins, which
+ * is a legacy irq, vector already reserved.
+ * Nothing to do in this case.
+ * case 4: irq with speical type (not from IOAPIC/MSI)
+ * These irq value are pre-defined for Timer, IPI, Spurious etc,
+ * which is listed in irq_static_mappings[].
+ * Nothing to do in this case.
+ *
+ * return value: valid irq if successfully prepared, otherwise
+IRQ_INVALID */ static uint32_t prepare_irq_resource(uint32_t req_irq)
Prepare is not very clear...
What you did include:
Conditionally allocate irq,
Allocate vector.

It is kind of initialize_irq_desc?

+{
+ uint32_t irq = req_irq;
+ uint32_t vr;
+
+ if (irq >= NR_IRQS) {
"> NR_IRQ" is very confusing.. Do you mean INVALID_IRQ ?
Do not assume INVALID_IRQ is larger than NR_IRQS, this may be not true.

+ /* case 1: */
+ irq = alloc_irq_desc(irq);
+ if (irq == IRQ_INVALID) {
+ return IRQ_INVALID;
+ }
+
+ vr = alloc_vector_for_irq(irq);
+ if (vr == VECTOR_INVALID) {
+ free_irq_desc(irq);
+ return IRQ_INVALID;
+ }
+ } else if (irq_is_gsi(irq) && irq >= NR_LEGACY_IRQ) {
+ /* case 2: */
+ vr = alloc_vector_for_irq(irq);
+ if (vr == VECTOR_INVALID) {
+ return IRQ_INVALID;
+ }
+ }
+
+ return irq;
+}
+
+/* release irq & vector if dynamically allocated */ static void
+release_irq_resource(uint32_t irq) {
+ struct irq_desc *desc = &irq_desc_array[irq];
+
+ if (irq_is_gsi(irq) && irq >= NR_LEGACY_IRQ) {
+ free_vector_for_irq(irq);
+ } else if (!irq_is_gsi(irq) && desc->vector <= VECTOR_DYNAMIC_END) {
+ free_vector_for_irq(irq);
+ free_irq_desc(irq);
}
}

@@ -166,78 +264,29 @@ static void disable_pic_irq(void)
pio_write8(0xffU, 0x21U);
}

-int32_t request_irq(uint32_t irq_arg,
+int32_t request_irq(uint32_t req_irq,
irq_action_t action_fn,
void *priv_data,
const char *name)
{
struct irq_desc *desc;
- uint32_t irq = irq_arg, vector;
+ uint32_t irq;
spinlock_rflags;

- /* ======================================================
- * This is low level ISR handler registering function
- * case: irq = IRQ_INVALID
- * caller did not know which irq to use, and want system to
- * allocate available irq for it. These irq are in range:
- * nr_gsi ~ NR_IRQS
- * a irq will be allocated and the vector will be assigned to this
- * irq automatically.
- *
- * case: irq >=0 and irq < nr_gsi
- * caller want to add device ISR handler into ioapic pins.
- * two kind of devices: legacy device and PCI device with INTx
- * a vector will automatically assigned.
- *
- * case: irq with speical type (not from IOAPIC/MSI)
- * These irq value are pre-defined for Timer, IPI, Spurious etc
- * vectors are pre-defined also
- *
- * return value: pinned irq and assigned vector for this irq.
- * caller can use this irq to enable/disable/mask/unmask interrupt
- * and if this irq is for:
- * GSI legacy: nothing to do for legacy irq, already initialized
- * GSI other: need to progam PCI INTx to match this irq pin
- * MSI: caller need program vector to PCI device
- *
- * =====================================================
- */
-
- /* HV select a irq for device if irq < 0
- * this vector/irq match to APCI DSDT or PCI INTx/MSI
- */
- if (irq == IRQ_INVALID) {
- irq = alloc_irq();
- } else {
- irq = irq_mark_used(irq);
- }
-
+ irq = prepare_irq_resource(req_irq);
if (irq >= NR_IRQS) {
- pr_err("failed to assign IRQ");
+ pr_err("%s: failed to prepare irq", __func__);
return -EINVAL;
}

desc = &irq_desc_array[irq];
+
+ spinlock_irqsave_obtain(&desc->irq_lock);
if (desc->irq_handler == NULL) {
desc->irq_handler = common_handler_edge;
}

- vector = desc->vector;
- if (vector >= VECTOR_FIXED_START &&
- vector <= VECTOR_FIXED_END) {
- irq_desc_set_vector(irq, vector);
- } else if (vector > NR_MAX_VECTOR) {
- irq_desc_alloc_vector(irq);
- }
-
- if (desc->vector == VECTOR_INVALID) {
- pr_err("the input vector is not correct");
- /* FIXME: free allocated irq */
- return -EINVAL;
- }
-
if (desc->action == NULL) {
- spinlock_irqsave_obtain(&desc->irq_lock);
desc->priv_data = priv_data;
desc->action = action_fn;

@@ -246,73 +295,22 @@ int32_t request_irq(uint32_t irq_arg,
*/
(void)strcpy_s(desc->name, 32U, name);

- spinlock_irqrestore_release(&desc->irq_lock);
} else {
+ spinlock_irqrestore_release(&desc->irq_lock);
+ release_irq_resource(irq);
pr_err("%s: request irq(%u) vr(%u) for %s failed,\
already requested", __func__,
irq, irq_to_vector(irq), name);
return -EBUSY;
}

+ spinlock_irqrestore_release(&desc->irq_lock);
dev_dbg(ACRN_DBG_IRQ, "[%s] %s irq%d vr:0x%x",
__func__, desc->name, irq, desc->vector);

return (int32_t)irq;
}

-/* it is safe to call irq_desc_alloc_vector multiple times*/ -uint32_t
irq_desc_alloc_vector(uint32_t irq) -{
- uint32_t vr = VECTOR_INVALID;
- struct irq_desc *desc;
-
- spinlock_rflags;
-
- /* irq should be always available at this time */
- if (irq >= NR_IRQS) {
- return VECTOR_INVALID;
- }
-
- desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
- if (desc->vector != VECTOR_INVALID) {
- /* already allocated a vector */
- goto OUT;
- }
-
- /* FLAT mode, a irq connected to every cpu's same vector */
- vr = find_available_vector();
- if (vr > NR_MAX_VECTOR) {
- pr_err("no vector found for irq[%d]", irq);
- goto OUT;
- }
- local_irq_desc_set_vector(irq, vr);
-OUT:
- spinlock_irqrestore_release(&desc->irq_lock);
- return vr;
-}
-
-void irq_desc_try_free_vector(uint32_t irq) -{
- struct irq_desc *desc;
-
- spinlock_rflags;
-
- /* legacy irq's vector is reserved and should not be freed */
- if ((irq >= NR_IRQS) || (irq < NR_LEGACY_IRQ)) {
- return;
- }
-
- desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
- if (desc->action == NULL) {
- _irq_desc_free_vector(irq);
- }
-
- spinlock_irqrestore_release(&desc->irq_lock);
-
-}
-
uint32_t irq_to_vector(uint32_t irq)
{
if (irq < NR_IRQS) {
@@ -577,9 +575,9 @@ void free_irq(uint32_t irq)
desc->action = NULL;
desc->priv_data = NULL;
memset(desc->name, '\0', 32U);
+ release_irq_resource(irq);

spinlock_irqrestore_release(&desc->irq_lock);
- irq_desc_try_free_vector(desc->irq);
}

#ifdef HV_DEBUG
diff --git a/hypervisor/include/arch/x86/irq.h
b/hypervisor/include/arch/x86/irq.h
index 60c276c8..b67d9b8b 100644
--- a/hypervisor/include/arch/x86/irq.h
+++ b/hypervisor/include/arch/x86/irq.h
@@ -67,6 +67,13 @@ void setup_notification(void); typedef void
(*spurious_handler_t)(uint32_t vector); extern spurious_handler_t
spurious_handler;

+uint32_t alloc_irq_desc(uint32_t req_irq); void free_irq_desc(uint32_t
+irq); uint32_t alloc_vector_for_irq(uint32_t req_irq); void
+free_vector_for_irq(uint32_t irq);
+
+uint32_t irq_to_vector(uint32_t irq);
+
/*
* Some MSI message definitions
*/
diff --git a/hypervisor/include/common/irq.h
b/hypervisor/include/common/irq.h index 627b31dc..ac6edbdf 100644
--- a/hypervisor/include/common/irq.h
+++ b/hypervisor/include/common/irq.h
@@ -43,13 +43,6 @@ struct irq_desc {
uint64_t irq_lost_cnt;
};

-uint32_t irq_mark_used(uint32_t irq);
-
-uint32_t irq_desc_alloc_vector(uint32_t irq); -void
irq_desc_try_free_vector(uint32_t irq);
-
-uint32_t irq_to_vector(uint32_t irq);
-
int32_t request_irq(uint32_t irq,
irq_action_t action_fn,
void *priv_data,
--
2.18.0



Re: [PATCH v4] HV: fix "missing for discarded return value" for vm related api

Junjie Mao
 

Hi Eddie,

Eddie Dong <eddie.dong@...> writes:

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Wednesday, August 15, 2018 10:36 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v4] HV: fix "missing for discarded return value"
for vm related api

- add handler if prepare_vm() failed;

- replace assert in start_vm()/prepare_vm() with return errno;

- return errno in start_vm()/reset_vm()/prepare_vm() so that caller
could handle it;

changelog:
v3 -> v4: add fix in partition mode;
replace return -1 with return errno;
v2 -> v3: replace panic with pr_fatal and return directly
if prepare_vm0() failed;
v1 -> v2: panic if prepare_vm0() failed instead of reboot system;

Signed-off-by: Victor Sun <victor.sun@...>
---
hypervisor/arch/x86/cpu.c | 10 ++++++++--
hypervisor/arch/x86/guest/vm.c | 21 +++++++++++----------
hypervisor/common/hypercall.c | 3 +--
3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index
4d7bf08..f63909a 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -502,7 +502,10 @@ static void bsp_boot_post(void)

exec_vmxon_instr(BOOT_CPU_ID);

- prepare_vm(BOOT_CPU_ID);
+ if (prepare_vm(BOOT_CPU_ID) != 0) {
+ pr_fatal("Prepare VM0 failed!");
+ return;
+ }

default_idle();

@@ -579,7 +582,10 @@ static void cpu_secondary_post(void)
exec_vmxon_instr(get_cpu_id());

#ifdef CONFIG_PARTITION_MODE
- prepare_vm(get_cpu_id());
+ if (prepare_vm(get_cpu_id()) != 0) {
+ pr_fatal("Prepare VM failed!");
+ return;
+ }
#endif

default_idle();
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index f2efbf9..a8fbaa7 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -340,9 +340,12 @@ int start_vm(struct vm *vm)

/* Only start BSP (vid = 0) and let BSP start other APs */
vcpu = vcpu_from_vid(vm, 0U);
- ASSERT(vcpu != NULL, "vm%d, vcpu0", vm->vm_id);
+ if (vcpu == NULL) {
+ return -EINVAL;
+ }
Can u fix all the caller side to handle the error properly?

schedule_vcpu(vcpu);

+ pr_acrnlog("Start VM%d", vm->vm_id);
return 0;
}

@@ -355,7 +358,7 @@ int reset_vm(struct vm *vm)
struct vcpu *vcpu = NULL;

if (vm->state != VM_PAUSED)
- return -1;
+ return -EINVAL;

foreach_vcpu(i, vm, vcpu) {
reset_vcpu(vcpu);
@@ -369,8 +372,7 @@ int reset_vm(struct vm *vm)
destroy_secure_world(vm, false);
vm->sworld_control.flag.active = 0UL;

- start_vm(vm);
- return 0;
+ return (start_vm(vm));
I remember MISRAC doesn't allow to combine 2 operations together here.

Returning the result of an expression is fine from the p.o.v. of MISRA
C. Have I missed anything? Thanks.

--
Best Regards
Junjie Mao


Re: [PATCH] DM: virtio rpmb backend driver updates

Zhu, Bing
 

Reviewed.
Patch is good to me.

-----Original Message-----
From: Huang, Yang
Sent: Tuesday, August 14, 2018 2:46 AM
To: acrn-dev@...
Cc: Deng, Wei A <wei.a.deng@...>; Huang, Yang <yang.huang@...>;
Dong, Eddie <eddie.dong@...>; Zhu, Bing <bing.zhu@...>; Wang, Kai
Z <kai.z.wang@...>; Xu, Anthony <anthony.xu@...>
Subject: [PATCH] DM: virtio rpmb backend driver updates

From: Deng Wei <wei.a.deng@...>

RPMB frontend driver in UOS kernel has fixed unstable issue, which requires BE for
update as well. E.g. structure adjustment, definition modification and so on.

Signed-off-by: Deng Wei <wei.a.deng@...>
Signed-off-by: Huang Yang <yang.huang@...>
---
devicemodel/hw/pci/virtio/virtio_rpmb.c | 224 +++++++-------------------------
devicemodel/include/rpmb.h | 2 +-
2 files changed, 47 insertions(+), 179 deletions(-)

diff --git a/devicemodel/hw/pci/virtio/virtio_rpmb.c
b/devicemodel/hw/pci/virtio/virtio_rpmb.c
index ec36524..ac05f91 100644
--- a/devicemodel/hw/pci/virtio/virtio_rpmb.c
+++ b/devicemodel/hw/pci/virtio/virtio_rpmb.c
@@ -54,6 +54,7 @@
#include "rpmb_backend.h"

#define VIRTIO_RPMB_RINGSZ 64
+#define VIRTIO_RPMB_MAXSEGS 5
#define ADDR_NOT_PRESENT -2

static int virtio_rpmb_debug = 1;
@@ -78,159 +79,15 @@ struct virtio_rpmb { struct virtio_rpmb_ioctl_cmd {
unsigned int cmd; /*ioctl cmd*/
int result; /* result for ioctl cmd*/
-};
-
-struct virtio_rpmb_ioc_seq_cmd {
- struct virtio_rpmb_ioctl_cmd ioc;
- union {
- __u64 num_of_cmds; /*num of seq cmds*/
- __u64 seq_base;
- };
+ __u8 target; /* for emmc */
+ __u8 reserved[3]; /* not used */
};

struct virtio_rpmb_ioc_seq_data {
- struct virtio_rpmb_ioc_seq_cmd seq_cmd;
+ __u64 num_of_cmds; /*num of seq cmds*/
struct rpmb_cmd cmds[SEQ_CMD_MAX + 1]; };

-/*
- * get start address of seq cmds.
- */
-static struct rpmb_cmd *
-virtio_rpmb_get_seq_cmds(void *pdata)
-{
- struct virtio_rpmb_ioc_seq_data *seq_data;
-
- if (!pdata) {
- DPRINTF(("error, invalid args!\n"));
- return NULL;
- }
-
- seq_data = (struct virtio_rpmb_ioc_seq_data *)pdata;
-
- return (struct rpmb_cmd *)&seq_data->cmds[0];
-}
-
-/*
- * get address of seq specail frames.
- * index: the index of cmds.
- */
-static struct rpmb_frame *
-virtio_rpmb_get_seq_frames(void *pdata, __u32 index) -{
- struct virtio_rpmb_ioc_seq_data *seq_data;
- struct rpmb_frame *frames;
- struct rpmb_cmd *cmds, *cmd;
- __u64 ncmds;
- __u32 offset = 0;
- __u32 num = 0;
-
- if (!pdata || index > SEQ_CMD_MAX) {
- DPRINTF(("error, invalid args!\n"));
- return NULL;
- }
-
- seq_data = (struct virtio_rpmb_ioc_seq_data *)pdata;
-
- /* get number of cmds.*/
- ncmds = seq_data->seq_cmd.num_of_cmds;
- if (ncmds > SEQ_CMD_MAX) {
- DPRINTF(("error, ncmds(%llu) > max\n", ncmds));
- return NULL;
- }
-
- /* get start address of cmds.*/
- cmds = virtio_rpmb_get_seq_cmds(pdata);
- if (!cmds) {
- DPRINTF(("fail to get seq cmds.\n"));
- return NULL;
- }
-
- /* get start address of frames.*/
- frames = (struct rpmb_frame *)&seq_data->cmds[ncmds + 1];
- if (!frames) {
- DPRINTF(("error, invalid frames ptr.\n"));
- return NULL;
- }
-
- for (num = 0; num < index; num++) {
- cmd = &cmds[num];
- if (!cmd) {
- DPRINTF(("error, invalid cmd ptr.\n"));
- return NULL;
- }
- offset += cmd->nframes;
- }
-
- return (struct rpmb_frame *)&frames[offset];
-}
-
-/*
- * get address of all seq data.
- */
-static void *
-virtio_rpmb_get_seq_data(void *pdata)
-{
- struct virtio_rpmb_ioc_seq_data *seq_data;
-
- if (!pdata) {
- DPRINTF(("error, invalid args!\n"));
- return NULL;
- }
-
- seq_data = (struct virtio_rpmb_ioc_seq_data *)pdata;
-
- return (void *)&seq_data->seq_cmd.seq_base;
-}
-
-/*
- * Address space is different between
- * SOS VBS-U and UOS kernel.
- * Update frames_ptr to current space.
- */
-static int
-virtio_rpmb_map_seq_frames(struct iovec *iov) -{
- struct virtio_rpmb_ioc_seq_cmd *seq_cmd = NULL;
- struct rpmb_cmd *cmds, *cmd;
- __u64 index;
-
- if (!iov) {
- DPRINTF(("error, invalid arg!\n"));
- return -1;
- }
-
- seq_cmd = (struct virtio_rpmb_ioc_seq_cmd *)(iov->iov_base);
- if (!seq_cmd || seq_cmd->num_of_cmds > SEQ_CMD_MAX) {
- DPRINTF(("found invalid data.\n"));
- return -1;
- }
-
- /* get start address of cmds.*/
- cmds = virtio_rpmb_get_seq_cmds(iov->iov_base);
- if (!cmds) {
- DPRINTF(("fail to get seq cmds.\n"));
- return -1;
- }
-
- /* set frames_ptr to VBS-U space.*/
- for (index = 0; index < seq_cmd->num_of_cmds; index++) {
- cmd = &cmds[index];
- if (!cmd) {
- DPRINTF(("error, invalid cmd ptr.\n"));
- return -1;
- }
- /* set frames address.*/
- cmd->frames = virtio_rpmb_get_seq_frames(iov->iov_base, index);
- if (!cmd->frames) {
- DPRINTF(("fail to get frames[%llu] ptr!\n", index));
- return -1;
- }
- }
-
- return 0;
-}
-
static int
rpmb_check_mac(__u8 *key, struct rpmb_frame *frames, __u8 frame_cnt) { @@
-501,35 +358,50 @@ rpmb_get_blocks(void) }

static int
-virtio_rpmb_seq_handler(struct virtio_rpmb *rpmb, struct iovec *iov)
+virtio_rpmb_seq_handler(struct virtio_rpmb *rpmb, struct iovec *iov,
+ int n, int *tlen)
{
struct virtio_rpmb_ioctl_cmd *ioc = NULL;
+ struct virtio_rpmb_ioc_seq_data *seq;
+ struct rpmb_frame *frames;
void *pdata;
int rc;
+ int i;
+ int size;

- if (!rpmb || !iov) {
+ assert(n >= 3 && n <= VIRTIO_RPMB_MAXSEGS);
+ if (!rpmb || !iov || !tlen) {
DPRINTF(("found invalid args!!!\n"));
return -1;
}

- /* update frames_ptr to curent space*/
- rc = virtio_rpmb_map_seq_frames(iov);
- if (rc) {
- DPRINTF(("fail to map seq frames\n"));
- return rc;
- }
-
- ioc = (struct virtio_rpmb_ioctl_cmd *)(iov->iov_base);
+ ioc = (struct virtio_rpmb_ioctl_cmd *)(iov[0].iov_base);
if (!ioc) {
DPRINTF(("error, get ioc is NULL!\n"));
return -1;
}
+ *tlen = iov[0].iov_len;

- pdata = virtio_rpmb_get_seq_data(iov->iov_base);
- if (!pdata) {
+ seq = (struct virtio_rpmb_ioc_seq_data *)(iov[1].iov_base);
+ if (!seq) {
DPRINTF(("fail to get seq data\n"));
return -1;
}
+ assert((n-2) == seq->num_of_cmds);
+ pdata = (void *)seq;
+
+ for (i = 2; i < n; i++) {
+ frames = (struct rpmb_frame *)(iov[i].iov_base);
+ if (!frames) {
+ DPRINTF(("fail to get frame data\n"));
+ return -1;
+ }
+
+ size = (seq->cmds[i-2].nframes ? :1)* sizeof(struct rpmb_frame);
+ assert(size == iov[i].iov_len);
+ seq->cmds[i-2].frames =
+ (struct rpmb_frame *)(iov[i].iov_base);
+ }

rc = rpmb_handler(ioc->cmd, pdata);
if (rc)
@@ -538,38 +410,34 @@ virtio_rpmb_seq_handler(struct virtio_rpmb *rpmb,
struct iovec *iov)
return rc;
}

+/*
+ * To meet the communication protocol from vRPMB FE,
+ * each time we will receive 3 or 4 or 5 iovs.
+ */
static void
virtio_rpmb_notify(void *base, struct virtio_vq_info *vq) {
- struct iovec iov;
- int len;
- __u16 idx;
+ struct iovec iov[VIRTIO_RPMB_MAXSEGS + 1];
+ int n;
+ int tlen = 0;
+ uint16_t idx;
struct virtio_rpmb *rpmb = (struct virtio_rpmb *)base;
struct virtio_rpmb_ioctl_cmd *ioc;

while (vq_has_descs(vq)) {
- vq_getchain(vq, &idx, &iov, 1, NULL);
-
- ioc = (struct virtio_rpmb_ioctl_cmd *)(iov.iov_base);
+ n = vq_getchain(vq, &idx, iov, VIRTIO_RPMB_MAXSEGS, NULL);
+ assert(n >= 3 && n <= VIRTIO_RPMB_MAXSEGS);

- switch (ioc->cmd) {
- case RPMB_IOC_SEQ_CMD:
- ioc->result = virtio_rpmb_seq_handler(rpmb, &iov);
- break;
-
- default:
- DPRINTF(("found unsupported ioctl(%u)!\n", ioc->cmd));
- ioc->result = -1;
- break;
- }
+ ioc = (struct virtio_rpmb_ioctl_cmd *)(iov[0].iov_base);
+ assert(RPMB_IOC_SEQ_CMD == ioc->cmd);

- len = iov.iov_len;
- assert(len > 0);
+ ioc->result = virtio_rpmb_seq_handler(rpmb, iov, n, &tlen);
+ assert(tlen > 0);

/*
* Release this chain and handle more
*/
- vq_relchain(vq, idx, len);
+ vq_relchain(vq, idx, tlen);
}
vq_endchains(vq, 1); /* Generate interrupt if appropriate. */
}
diff --git a/devicemodel/include/rpmb.h b/devicemodel/include/rpmb.h index
c8d9251..ab57708 100644
--- a/devicemodel/include/rpmb.h
+++ b/devicemodel/include/rpmb.h
@@ -188,7 +188,7 @@ int
rpmb_get_counter(__u8 mode, __u8 *key, __u32 *counter, __u16 *result);

#define RPMB_IOC_REQ_CMD _IOWR(0xB5, 80, struct rpmb_ioc_req_cmd) -
#define RPMB_IOC_SEQ_CMD _IOWR(0xB5, 81, struct rpmb_ioc_seq_cmd)
+#define RPMB_IOC_SEQ_CMD _IOWR(0xB5, 82, struct rpmb_ioc_seq_cmd)

__u16 rpmb_get_blocks(void);

--
2.7.4


Re: [PATCH v4] HV: fix "missing for discarded return value" for vm related api

Eddie Dong
 

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Wednesday, August 15, 2018 10:36 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v4] HV: fix "missing for discarded return value"
for vm related api

- add handler if prepare_vm() failed;

- replace assert in start_vm()/prepare_vm() with return errno;

- return errno in start_vm()/reset_vm()/prepare_vm() so that caller
could handle it;

changelog:
v3 -> v4: add fix in partition mode;
replace return -1 with return errno;
v2 -> v3: replace panic with pr_fatal and return directly
if prepare_vm0() failed;
v1 -> v2: panic if prepare_vm0() failed instead of reboot system;

Signed-off-by: Victor Sun <victor.sun@...>
---
hypervisor/arch/x86/cpu.c | 10 ++++++++--
hypervisor/arch/x86/guest/vm.c | 21 +++++++++++----------
hypervisor/common/hypercall.c | 3 +--
3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index
4d7bf08..f63909a 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -502,7 +502,10 @@ static void bsp_boot_post(void)

exec_vmxon_instr(BOOT_CPU_ID);

- prepare_vm(BOOT_CPU_ID);
+ if (prepare_vm(BOOT_CPU_ID) != 0) {
+ pr_fatal("Prepare VM0 failed!");
+ return;
+ }

default_idle();

@@ -579,7 +582,10 @@ static void cpu_secondary_post(void)
exec_vmxon_instr(get_cpu_id());

#ifdef CONFIG_PARTITION_MODE
- prepare_vm(get_cpu_id());
+ if (prepare_vm(get_cpu_id()) != 0) {
+ pr_fatal("Prepare VM failed!");
+ return;
+ }
#endif

default_idle();
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index f2efbf9..a8fbaa7 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -340,9 +340,12 @@ int start_vm(struct vm *vm)

/* Only start BSP (vid = 0) and let BSP start other APs */
vcpu = vcpu_from_vid(vm, 0U);
- ASSERT(vcpu != NULL, "vm%d, vcpu0", vm->vm_id);
+ if (vcpu == NULL) {
+ return -EINVAL;
+ }
Can u fix all the caller side to handle the error properly?

schedule_vcpu(vcpu);

+ pr_acrnlog("Start VM%d", vm->vm_id);
return 0;
}

@@ -355,7 +358,7 @@ int reset_vm(struct vm *vm)
struct vcpu *vcpu = NULL;

if (vm->state != VM_PAUSED)
- return -1;
+ return -EINVAL;

foreach_vcpu(i, vm, vcpu) {
reset_vcpu(vcpu);
@@ -369,8 +372,7 @@ int reset_vm(struct vm *vm)
destroy_secure_world(vm, false);
vm->sworld_control.flag.active = 0UL;

- start_vm(vm);
- return 0;
+ return (start_vm(vm));
I remember MISRAC doesn't allow to combine 2 operations together here.


}

/**
@@ -450,7 +452,10 @@ int prepare_vm(uint16_t pcpu_id)

if (is_vm_bsp) {
ret = create_vm(vm_desc, &vm);
- ASSERT(ret == 0, "VM creation failed!");
+ if (ret != 0) {
+ pr_fatal("VM creation failed!");
+ return ret;
+ }

mptable_build(vm);

@@ -461,9 +466,7 @@ int prepare_vm(uint16_t pcpu_id)
prepare_vcpu(vm, vm_desc->vm_pcpu_ids[i]);

/* start vm BSP automatically */
- start_vm(vm);
-
- pr_acrnlog("Start VM%x", vm_desc->vm_id);
+ ret = start_vm(vm);
}

return ret;
@@ -499,8 +502,6 @@ int prepare_vm0(void)
/* start vm0 BSP automatically */
err = start_vm(vm);

- pr_acrnlog("Start VM0");
-
return err;
}

diff --git a/hypervisor/common/hypercall.c
b/hypervisor/common/hypercall.c index fd4bc29..fdcfca0 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -280,8 +280,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
if ((target_vm == NULL) || is_vm0(target_vm))
return -1;

- reset_vm(target_vm);
- return 0;
+ return (reset_vm(target_vm));
}

int32_t hcall_assert_irqline(struct vm *vm, uint16_t vmid, uint64_t param)
--
2.7.4



Re: [PATCH] ipu: virtio-ipu4 as default IPU DM

Zhai, Edwin
 

Reviewed-by: Edwin Zhai <edwin.zhai@...>

On Tue, 14 Aug 2018, Yew, Chang Ching wrote:

Change-Id: I9dd504da79d31863be4c95bcec21fcc9640bd336
Signed-off-by: Yew, Chang Ching <chang.ching.yew@...>
---
devicemodel/samples/apl-mrb/launch_uos.sh | 74 +++++++++++++----------
1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/devicemodel/samples/apl-mrb/launch_uos.sh b/devicemodel/samples/apl-mrb/launch_uos.sh
index 8cadc40..b977780 100755
--- a/devicemodel/samples/apl-mrb/launch_uos.sh
+++ b/devicemodel/samples/apl-mrb/launch_uos.sh
@@ -1,5 +1,7 @@
#!/bin/bash

+ipu_passthrough=0
+
function launch_clearlinux()
{
if [ ! -f "/data/$5/$5.img" ]; then
@@ -54,22 +56,26 @@ echo "0000:00:0f.0" > /sys/bus/pci/devices/0000:00:0f.0/driver/unbind
echo "0000:00:0f.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0
-if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0
-# please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller
-# could get the same virtaul BDF as physical BDF
-if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

# for sd card passthrough - SDXC/MMC Host Controller 00:1b.0
@@ -210,22 +216,26 @@ echo "0000:00:18.0" > /sys/bus/pci/devices/0000:00:18.0/driver/unbind
echo "0000:00:18.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0
-if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0
-# please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller
-# could get the same virtaul BDF as physical BDF
-if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

#for memsize setting
--
2.18.0



Best Rgds,
Edwin


[PATCH 2/2] DM: Add boot option of "i915.enable_guc=0" to disable Guc on UOS new kernel

Zhao, Yakui
 

The guc boot option is refined on the new linux kernel. The boot option of
"i915.enable_guc=0" should be added in order to disable Guc instead of using
"enable_guc_loading/submission". But in order to use the same boot option on
multi kernel, both of them are kept.

Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
devicemodel/samples/apl-mrb/launch_uos.sh | 4 ++--
devicemodel/samples/nuc/launch_uos.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/devicemodel/samples/apl-mrb/launch_uos.sh b/devicemodel/samples/apl-mrb/launch_uos.sh
index 6595b6b..b624e5e 100755
--- a/devicemodel/samples/apl-mrb/launch_uos.sh
+++ b/devicemodel/samples/apl-mrb/launch_uos.sh
@@ -123,7 +123,7 @@ acrn-dm -A -m $mem_size -c $2$boot_GVT_option"$GVT_args" -s 0:0,hostbridge -s 1:
console=ttyS0 no_timer_check ignore_loglevel log_buf_len=16M \
consoleblank=0 tsc=reliable i915.avail_planes_per_pipe=$4 i915.enable_guc_loading=0 \
i915.enable_hangcheck=0 i915.nuclear_pageflip=1 \
- i915.enable_guc_submission=0" $vm_name
+ i915.enable_guc_submission=0 i915.enable_guc=0" $vm_name
}

function launch_android()
@@ -243,7 +243,7 @@ fi
kernel_cmdline_generic="maxcpus=$2 nohpet tsc=reliable intel_iommu=off \
androidboot.serialno=$ser \
i915.enable_rc6=1 i915.enable_fbc=1 i915.enable_guc_loading=0 i915.avail_planes_per_pipe=$4 \
- i915.enable_hangcheck=0 use_nuclear_flip=1 i915.enable_guc_submission=0"
+ i915.enable_hangcheck=0 use_nuclear_flip=1 i915.enable_guc_submission=0 i915.enable_guc=0"

boot_dev_flag=",b"
if [ $6 == 1 ];then
diff --git a/devicemodel/samples/nuc/launch_uos.sh b/devicemodel/samples/nuc/launch_uos.sh
index 4efe807..1f08d15 100755
--- a/devicemodel/samples/nuc/launch_uos.sh
+++ b/devicemodel/samples/nuc/launch_uos.sh
@@ -27,7 +27,7 @@ acrn-dm -A -m $mem_size -c $2 -s 0:0,hostbridge -s 1:0,lpc -l com1,stdio \
console=ttyS0 no_timer_check ignore_loglevel log_buf_len=16M \
consoleblank=0 tsc=reliable i915.avail_planes_per_pipe=$4 \
i915.enable_hangcheck=0 i915.nuclear_pageflip=1 i915.enable_guc_loading=0 \
- i915.enable_guc_submission=0" $vm_name
+ i915.enable_guc_submission=0 i915.enable_guc=0" $vm_name
}

# offline SOS CPUs except BSP before launch UOS
--
2.7.4


[PATCH 0/2] DM: Add the boot option to disable Guc on SOS/UOS

Zhao, Yakui
 

THis is the patch set that adds one boot option to disable Guc on SOS/UOS.
The guc boot option is refined on the new linux kernel. The boot option of
"i915.enable_guc=0" should be added in order to disable Guc instead of using
"enable_guc_loading/submission".

Zhao Yakui (2):
DM: Add the boot option of "i915.enable_guc=0" to disable guc on SOS
new kernel
DM: Add boot option of "i915.enable_guc=0" to disable Guc on UOS new
kernel

devicemodel/samples/apl-mrb/launch_uos.sh | 4 ++--
devicemodel/samples/apl-mrb/sos_bootargs_debug.txt | 2 +-
devicemodel/samples/apl-mrb/sos_bootargs_release.txt | 2 +-
devicemodel/samples/nuc/launch_uos.sh | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)

--
2.7.4


[PATCH 1/2] DM: Add the boot option of "i915.enable_guc=0" to disable guc on SOS new kernel

Zhao, Yakui
 

The guc boot option is refined on the new linux kernel. The boot option of
"i915.enable_guc=0" should be added in order to disable Guc instead of using
"enable_guc_loading/submission". But in order to use the same boot option on
multi kernel, both of them are kept.

Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
devicemodel/samples/apl-mrb/sos_bootargs_debug.txt | 2 +-
devicemodel/samples/apl-mrb/sos_bootargs_release.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/devicemodel/samples/apl-mrb/sos_bootargs_debug.txt b/devicemodel/samples/apl-mrb/sos_bootargs_debug.txt
index 53cdac0..1c0394a 100644
--- a/devicemodel/samples/apl-mrb/sos_bootargs_debug.txt
+++ b/devicemodel/samples/apl-mrb/sos_bootargs_debug.txt
@@ -1 +1 @@
-pci_devices_ignore=(0:18:2) console=tty0 console=ttyS0 i915.nuclear_pageflip=1 root=/dev/mmcblk1p1 rw rootwait quiet loglevel=3 no_timer_check consoleblank=0 i915.enable_initial_modeset=1 i915.tsd_init=7 i915.tsd_delay=2000 video=DP-1:d video=DP-2:d i915.avail_planes_per_pipe=0x01010F i915.domain_plane_owners=0x011111110000 i915.enable_guc_loading=0 i915.enable_guc_submission=0 i915.enable_preemption=1 i915.context_priority_mode=2 i915.enable_gvt=1 hvlog=2M@0x6de00000 reboot_panic=p,w cma=32M@0-
+pci_devices_ignore=(0:18:2) console=tty0 console=ttyS0 i915.nuclear_pageflip=1 root=/dev/mmcblk1p1 rw rootwait quiet loglevel=3 no_timer_check consoleblank=0 i915.enable_initial_modeset=1 i915.tsd_init=7 i915.tsd_delay=2000 video=DP-1:d video=DP-2:d i915.avail_planes_per_pipe=0x01010F i915.domain_plane_owners=0x011111110000 i915.enable_guc_loading=0 i915.enable_guc_submission=0 i915.enable_preemption=1 i915.context_priority_mode=2 i915.enable_gvt=1 i915.enable_guc=0 hvlog=2M@0x6de00000 reboot_panic=p,w cma=32M@0-
diff --git a/devicemodel/samples/apl-mrb/sos_bootargs_release.txt b/devicemodel/samples/apl-mrb/sos_bootargs_release.txt
index b31ea60..5c0d4da 100644
--- a/devicemodel/samples/apl-mrb/sos_bootargs_release.txt
+++ b/devicemodel/samples/apl-mrb/sos_bootargs_release.txt
@@ -1 +1 @@
-console=tty0 i915.nuclear_pageflip=1 root=/dev/mmcblk1p1 rw rootwait quiet loglevel=3 no_timer_check consoleblank=0 i915.enable_initial_modeset=1 i915.tsd_init=7 i915.tsd_delay=2000 video=DP-1:d video=DP-2:d i915.avail_planes_per_pipe=0x01010F i915.domain_plane_owners=0x011111110000 i915.enable_guc_loading=0 i915.enable_guc_submission=0 i915.enable_preemption=1 i915.context_priority_mode=2 i915.enable_gvt=1 cma=32M@0-
+console=tty0 i915.nuclear_pageflip=1 root=/dev/mmcblk1p1 rw rootwait quiet loglevel=3 no_timer_check consoleblank=0 i915.enable_initial_modeset=1 i915.tsd_init=7 i915.tsd_delay=2000 video=DP-1:d video=DP-2:d i915.avail_planes_per_pipe=0x01010F i915.domain_plane_owners=0x011111110000 i915.enable_guc_loading=0 i915.enable_guc_submission=0 i915.enable_preemption=1 i915.context_priority_mode=2 i915.enable_gvt=1 i915.enable_guc=0 cma=32M@0-
--
2.7.4


Re: [PATCH] ipu: virtio-ipu4 as default IPU DM

Minggui Cao
 

Reviewed-by: Minggui Cao <minggui.cao@...>

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yew, Chang Ching
Sent: Tuesday, August 14, 2018 4:55 PM
To: acrn-dev@...
Cc: Yew, Chang Ching <chang.ching.yew@...>; Bandi, Kushal <kushal.bandi@...>; Yu, Ong Hock <ong.hock.yu@...>; Gopalakrishnan, Karthik L <karthik.l.gopalakrishnan@...>; Gao, Shiqing <shiqing.gao@...>
Subject: [acrn-dev] [PATCH] ipu: virtio-ipu4 as default IPU DM

Change-Id: I9dd504da79d31863be4c95bcec21fcc9640bd336
Signed-off-by: Yew, Chang Ching <chang.ching.yew@...>
---
devicemodel/samples/apl-mrb/launch_uos.sh | 74 +++++++++++++----------
1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/devicemodel/samples/apl-mrb/launch_uos.sh b/devicemodel/samples/apl-mrb/launch_uos.sh
index 8cadc40..b977780 100755
--- a/devicemodel/samples/apl-mrb/launch_uos.sh
+++ b/devicemodel/samples/apl-mrb/launch_uos.sh
@@ -1,5 +1,7 @@
#!/bin/bash

+ipu_passthrough=0
+
function launch_clearlinux()
{
if [ ! -f "/data/$5/$5.img" ]; then
@@ -54,22 +56,26 @@ echo "0000:00:0f.0" > /sys/bus/pci/devices/0000:00:0f.0/driver/unbind
echo "0000:00:0f.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0 -if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0 -# please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller -# could get the same virtaul BDF as physical BDF -if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

# for sd card passthrough - SDXC/MMC Host Controller 00:1b.0 @@ -210,22 +216,26 @@ echo "0000:00:18.0" > /sys/bus/pci/devices/0000:00:18.0/driver/unbind
echo "0000:00:18.0" > /sys/bus/pci/drivers/pci-stub/bind

boot_ipu_option=""
-# for ipu passthrough - ipu device 0:3.0 -if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
- echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
- echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
-fi
-
-# for ipu passthrough - ipu related i2c 0:16.0 -# please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller -# could get the same virtaul BDF as physical BDF -if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
- echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
- echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
- echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
- boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+if [ $ipu_passthrough == 1 ];then
+ # for ipu passthrough - ipu device 0:3.0
+ if [ -d "/sys/bus/pci/devices/0000:00:03.0" ]; then
+ echo "8086 5a88" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:03.0" > /sys/bus/pci/devices/0000:00:03.0/driver/unbind
+ echo "0000:00:03.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 12,passthru,0/3/0 "
+ fi
+
+ # for ipu passthrough - ipu related i2c 0:16.0
+ # please use virtual slot 22 for i2c 0:16.0 to make sure that the i2c controller
+ # could get the same virtaul BDF as physical BDF
+ if [ -d "/sys/bus/pci/devices/0000:00:16.0" ]; then
+ echo "8086 5aac" > /sys/bus/pci/drivers/pci-stub/new_id
+ echo "0000:00:16.0" > /sys/bus/pci/devices/0000:00:16.0/driver/unbind
+ echo "0000:00:16.0" > /sys/bus/pci/drivers/pci-stub/bind
+ boot_ipu_option="$boot_ipu_option"" -s 22,passthru,0/16/0 "
+ fi
+else
+ boot_ipu_option="$boot_ipu_option"" -s 21,virtio-ipu4 "
fi

#for memsize setting
--
2.18.0


[PATCH] hv: Add vrtc emulation support for ACRN partition mode

Grandhi, Sainath
 

This patch adds code to support read-only RTC support for guests
run by partition mode ACRN. It supports RW for CMOS address port 0x70
and RO for CMOS data port 0x71. Reads to CMOS RAM offsets are fetched
by reading CMOS h/w directly and writes to CMOS offsets are discarded.

Signed-off-by: Sainath Grandhi <sainath.grandhi@...>
---
hypervisor/Makefile | 1 +
hypervisor/arch/x86/guest/vm.c | 3 +-
hypervisor/dm/vrtc.c | 109 +++++++++++++++++++++++++++++++++
hypervisor/include/arch/x86/guest/vm.h | 4 ++
4 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644 hypervisor/dm/vrtc.c

diff --git a/hypervisor/Makefile b/hypervisor/Makefile
index d3e14f76..e3022132 100644
--- a/hypervisor/Makefile
+++ b/hypervisor/Makefile
@@ -177,6 +177,7 @@ C_SRCS += dm/vioapic.c
ifeq ($(CONFIG_PARTITION_MODE),y)
C_SRCS += $(wildcard dm/vpci/*.c)
C_SRCS += $(wildcard partition/*.c)
+C_SRCS += dm/vrtc.c
endif

C_SRCS += bsp/$(CONFIG_PLATFORM)/vm_description.c
diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c
index f2efbf9e..5c7c6e16 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -205,7 +205,7 @@ int create_vm(struct vm_description *vm_desc, struct vm **rtn_vm)
if (vm_desc->vm_vuart) {
vm->vuart = vuart_init(vm);
}
-
+ vm->vrtc = vrtc_init(vm);
vpci_init(vm);
#endif

@@ -317,6 +317,7 @@ int shutdown_vm(struct vm *vm)

#ifdef CONFIG_PARTITION_MODE
vpci_cleanup(vm);
+ vrtc_deinit(vm);
#endif

free(vm->hw.vcpu_array);
diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
new file mode 100644
index 00000000..bd3a8ef2
--- /dev/null
+++ b/hypervisor/dm/vrtc.c
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2018 Intel Corporation. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include <hypervisor.h>
+#include <hv_lib.h>
+#include <acrn_common.h>
+#include <hv_arch.h>
+#include <hv_debug.h>
+
+#define CMOS_ADDR_PORT 0x70U
+#define CMOS_DATA_PORT 0x71U
+
+#define RTC_STATUSA 0x0AU /* status register A */
+#define RTCSA_TUP 0x80U /* time update, don't look now */
+
+struct vrtc {
+ struct vm *vm;
+ uint8_t addr; /* RTC register to read or write */
+};
+
+static spinlock_t cmos_lock = { .head = 0U, .tail = 0U };
+
+static uint8_t cmos_read(uint8_t addr)
+{
+ pio_write8(addr, CMOS_ADDR_PORT);
+ return pio_read8(CMOS_DATA_PORT);
+}
+
+static bool cmos_update_in_progress(void)
+{
+ return (cmos_read(RTC_STATUSA) & RTCSA_TUP)?1:0;
+}
+
+uint8_t cmos_get_reg_val(uint8_t addr)
+{
+ uint8_t reg;
+ int tries = 2000U;
+
+ spinlock_obtain(&cmos_lock);
+
+ /* Make sure an update isn't in progress */
+ while (cmos_update_in_progress() && tries--)
+ ;
+
+ reg = cmos_read(addr);
+
+ spinlock_release(&cmos_lock);
+ return reg;
+}
+
+static uint32_t vrtc_read(__unused struct vm_io_handler *hdlr, struct vm *vm,
+ uint16_t addr, __unused size_t width)
+{
+ struct vrtc *vrtc = vm->vrtc;
+ uint8_t reg;
+ uint8_t offset;
+
+ offset = vrtc->addr;
+
+ if (addr == CMOS_ADDR_PORT) {
+ return vrtc->addr;
+ }
+
+ reg = cmos_get_reg_val(offset);
+ return reg;
+}
+
+static void vrtc_write(__unused struct vm_io_handler *hdlr, struct vm *vm, uint16_t addr,
+ size_t width, uint32_t value)
+{
+ struct vrtc *vrtc = vm->vrtc;
+
+
+ if (width != 1U)
+ return;
+
+ if (addr == CMOS_ADDR_PORT) {
+ vrtc->addr = value & 0x7FU;
+ }
+ return;
+}
+
+void *vrtc_init(struct vm *vm)
+{
+ struct vrtc *vrtc;
+ struct vm_io_range range = {
+ .flags = IO_ATTR_RW, .base = CMOS_ADDR_PORT, .len = 2U};
+
+ vrtc = calloc(1U, sizeof(struct vrtc));
+ ASSERT(vrtc != NULL, "");
+
+ vrtc->vm = vm;
+ register_io_emulation_handler(vm, &range, vrtc_read, vrtc_write);
+
+ return vrtc;
+}
+
+void vrtc_deinit(struct vm *vm)
+{
+ if (vm->vrtc == NULL) {
+ return;
+ }
+
+ free(vm->vrtc);
+ vm->vrtc = NULL;
+}
diff --git a/hypervisor/include/arch/x86/guest/vm.h b/hypervisor/include/arch/x86/guest/vm.h
index 800ab4df..9f5c8a11 100644
--- a/hypervisor/include/arch/x86/guest/vm.h
+++ b/hypervisor/include/arch/x86/guest/vm.h
@@ -166,6 +166,7 @@ struct vm {
#ifdef CONFIG_PARTITION_MODE
struct vm_description *vm_desc;
struct vpci vpci;
+ void *vrtc;
#endif
};

@@ -275,5 +276,8 @@ struct pcpu_vm_desc_mapping {
bool is_bsp;
};
extern const struct pcpu_vm_desc_mapping pcpu_vm_desc_map[];
+
+void *vrtc_init(struct vm *vm);
+void vrtc_deinit(struct vm *vm);
#endif
#endif /* VM_H_ */
--
2.14.1


[PATCH] HV: remove 'spinlock_rfags' declaration

Yonghua Huang
 

- remove the hardcoding loccal variable name 'cpu_int_vale'

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/arch/x86/ioapic.c | 13 ++++---
hypervisor/arch/x86/irq.c | 73 ++++++++++++++++++---------------------
hypervisor/common/ptdev.c | 25 +++++++-------
hypervisor/debug/logmsg.c | 23 ++++++------
hypervisor/include/arch/x86/cpu.h | 15 +++-----
hypervisor/include/lib/spinlock.h | 15 ++++----
6 files changed, 74 insertions(+), 90 deletions(-)

diff --git a/hypervisor/arch/x86/ioapic.c b/hypervisor/arch/x86/ioapic.c
index 954905b..20a3ee5 100644
--- a/hypervisor/arch/x86/ioapic.c
+++ b/hypervisor/arch/x86/ioapic.c
@@ -93,17 +93,16 @@ static inline uint32_t
ioapic_read_reg32(const void *ioapic_base, const uint32_t offset)
{
uint32_t v;
+ uint64_t rflags = 0;

- spinlock_rflags;
-
- spinlock_irqsave_obtain(&ioapic_lock);
+ spinlock_irqsave_obtain(&ioapic_lock, &rflags);

/* Write IOREGSEL */
mmio_write32(offset, (void *)ioapic_base + IOAPIC_REGSEL);
/* Read IOWIN */
v = mmio_read32((void *)ioapic_base + IOAPIC_WINDOW);

- spinlock_irqrestore_release(&ioapic_lock);
+ spinlock_irqrestore_release(&ioapic_lock, rflags);
return v;
}

@@ -111,16 +110,16 @@ static inline void
ioapic_write_reg32(const void *ioapic_base,
const uint32_t offset, const uint32_t value)
{
- spinlock_rflags;
+ uint64_t rflags = 0;

- spinlock_irqsave_obtain(&ioapic_lock);
+ spinlock_irqsave_obtain(&ioapic_lock, &rflags);

/* Write IOREGSEL */
mmio_write32(offset, (void *)ioapic_base + IOAPIC_REGSEL);
/* Write IOWIN */
mmio_write32(value, (void *)ioapic_base + IOAPIC_WINDOW);

- spinlock_irqrestore_release(&ioapic_lock);
+ spinlock_irqrestore_release(&ioapic_lock, rflags);
}

static inline uint64_t
diff --git a/hypervisor/arch/x86/irq.c b/hypervisor/arch/x86/irq.c
index 3d9c557..01976b5 100644
--- a/hypervisor/arch/x86/irq.c
+++ b/hypervisor/arch/x86/irq.c
@@ -67,20 +67,19 @@ static uint32_t find_available_vector()
*/
uint32_t irq_mark_used(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return IRQ_INVALID;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
}
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
return irq;
}

@@ -91,19 +90,19 @@ uint32_t irq_mark_used(uint32_t irq)
static uint32_t alloc_irq(void)
{
uint32_t i;
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;

for (i = irq_gsi_num(); i < NR_IRQS; i++) {
desc = &irq_desc_array[i];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->used == IRQ_NOT_ASSIGNED) {
desc->used = IRQ_ASSIGNED;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
break;
}
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}
return (i == NR_IRQS) ? IRQ_INVALID : i;
}
@@ -121,15 +120,14 @@ static void local_irq_desc_set_vector(uint32_t irq, uint32_t vr)
/* lock version of set vector */
static void irq_desc_set_vector(uint32_t irq, uint32_t vr)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
vector_to_irq[vr] = irq;
desc->vector = vr;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}

/* used with holding irq_lock outside */
@@ -173,7 +171,7 @@ int32_t request_irq(uint32_t irq_arg,
{
struct irq_desc *desc;
uint32_t irq = irq_arg, vector;
- spinlock_rflags;
+ uint64_t rflags = 0;

/* ======================================================
* This is low level ISR handler registering function
@@ -237,7 +235,7 @@ int32_t request_irq(uint32_t irq_arg,
}

if (desc->action == NULL) {
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->priv_data = priv_data;
desc->action = action_fn;

@@ -246,7 +244,7 @@ int32_t request_irq(uint32_t irq_arg,
*/
(void)strcpy_s(desc->name, 32U, name);

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
} else {
pr_err("%s: request irq(%u) vr(%u) for %s failed,\
already requested", __func__,
@@ -264,9 +262,9 @@ int32_t request_irq(uint32_t irq_arg,
uint32_t irq_desc_alloc_vector(uint32_t irq)
{
uint32_t vr = VECTOR_INVALID;
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;

/* irq should be always available at this time */
if (irq >= NR_IRQS) {
@@ -274,7 +272,7 @@ uint32_t irq_desc_alloc_vector(uint32_t irq)
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->vector != VECTOR_INVALID) {
/* already allocated a vector */
goto OUT;
@@ -288,28 +286,27 @@ uint32_t irq_desc_alloc_vector(uint32_t irq)
}
local_irq_desc_set_vector(irq, vr);
OUT:
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
return vr;
}

void irq_desc_try_free_vector(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
/* legacy irq's vector is reserved and should not be freed */
if ((irq >= NR_IRQS) || (irq < NR_LEGACY_IRQ)) {
return;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
if (desc->action == NULL) {
_irq_desc_free_vector(irq);
}

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

}

@@ -421,8 +418,8 @@ void partition_mode_dispatch_interrupt(struct intr_excp_ctx *ctx)
int handle_level_interrupt_common(struct irq_desc *desc,
__unused void *handler_data)
{
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock
@@ -434,7 +431,7 @@ int handle_level_interrupt_common(struct irq_desc *desc,
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* mask iopaic pin */
@@ -454,15 +451,15 @@ int handle_level_interrupt_common(struct irq_desc *desc,
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

return 0;
}

int common_handler_edge(struct irq_desc *desc, __unused void *handler_data)
{
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock
@@ -474,7 +471,7 @@ int common_handler_edge(struct irq_desc *desc, __unused void *handler_data)
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* Send EOI to LAPIC/IOAPIC IRR */
@@ -485,15 +482,15 @@ int common_handler_edge(struct irq_desc *desc, __unused void *handler_data)
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

return 0;
}

int common_dev_handler_level(struct irq_desc *desc, __unused void *handler_data)
{
+ uint64_t rflags = 0;
irq_action_t action = desc->action;
- spinlock_rflags;

/*
* give other Core a try to return without hold irq_lock
@@ -505,7 +502,7 @@ int common_dev_handler_level(struct irq_desc *desc, __unused void *handler_data)
return 0;
}

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->state = IRQ_DESC_IN_PROCESS;

/* mask iopaic pin */
@@ -521,7 +518,7 @@ int common_dev_handler_level(struct irq_desc *desc, __unused void *handler_data)
}

desc->state = IRQ_DESC_PENDING;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);

/* we did not unmask irq until guest EOI the vector */
return 0;
@@ -544,26 +541,24 @@ int quick_handler_nolock(struct irq_desc *desc, __unused void *handler_data)

void update_irq_handler(uint32_t irq, irq_handler_t func)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return;
}

desc = &irq_desc_array[irq];
- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);
desc->irq_handler = func;
- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
}

void free_irq(uint32_t irq)
{
+ uint64_t rflags = 0;
struct irq_desc *desc;

- spinlock_rflags;
-
if (irq >= NR_IRQS) {
return;
}
@@ -572,13 +567,13 @@ void free_irq(uint32_t irq)
dev_dbg(ACRN_DBG_IRQ, "[%s] %s irq%d vr:0x%x",
__func__, desc->name, irq, irq_to_vector(irq));

- spinlock_irqsave_obtain(&desc->irq_lock);
+ spinlock_irqsave_obtain(&desc->irq_lock, &rflags);

desc->action = NULL;
desc->priv_data = NULL;
memset(desc->name, '\0', 32U);

- spinlock_irqrestore_release(&desc->irq_lock);
+ spinlock_irqrestore_release(&desc->irq_lock, rflags);
irq_desc_try_free_vector(desc->irq);
}

diff --git a/hypervisor/common/ptdev.c b/hypervisor/common/ptdev.c
index ef12ce0..19d4800 100644
--- a/hypervisor/common/ptdev.c
+++ b/hypervisor/common/ptdev.c
@@ -30,26 +30,27 @@ static spinlock_t softirq_dev_lock;

static void ptdev_enqueue_softirq(struct ptdev_remapping_info *entry)
{
- spinlock_rflags;
+ uint64_t rflags = 0;
+
/* enqueue request in order, SOFTIRQ_PTDEV will pickup */
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);

/* avoid adding recursively */
list_del(&entry->softirq_node);
/* TODO: assert if entry already in list */
list_add_tail(&entry->softirq_node,
&softirq_dev_entry_list);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
fire_softirq(SOFTIRQ_PTDEV);
}

struct ptdev_remapping_info*
ptdev_dequeue_softirq(void)
{
+ uint64_t rflags = 0;
struct ptdev_remapping_info *entry = NULL;

- spinlock_rflags;
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);

if (!list_empty(&softirq_dev_entry_list)) {
entry = get_first_item(&softirq_dev_entry_list,
@@ -57,7 +58,7 @@ ptdev_dequeue_softirq(void)
list_del_init(&entry->softirq_node);
}

- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
return entry;
}

@@ -86,7 +87,7 @@ alloc_entry(struct vm *vm, enum ptdev_intr_type type)
void
release_entry(struct ptdev_remapping_info *entry)
{
- spinlock_rflags;
+ uint64_t rflags = 0;

/* remove entry from ptdev_list */
list_del_init(&entry->entry_node);
@@ -95,9 +96,9 @@ release_entry(struct ptdev_remapping_info *entry)
* remove entry from softirq list.the ptdev_lock
* is required before calling release_entry.
*/
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);
list_del_init(&entry->softirq_node);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);

free(entry);
}
@@ -146,7 +147,7 @@ ptdev_activate_entry(struct ptdev_remapping_info *entry, uint32_t phys_irq)
void
ptdev_deactivate_entry(struct ptdev_remapping_info *entry)
{
- spinlock_rflags;
+ uint64_t rflags = 0;

atomic_clear32(&entry->active, ACTIVE_FLAG);

@@ -154,9 +155,9 @@ ptdev_deactivate_entry(struct ptdev_remapping_info *entry)
entry->allocated_pirq = IRQ_INVALID;

/* remove from softirq list if added */
- spinlock_irqsave_obtain(&softirq_dev_lock);
+ spinlock_irqsave_obtain(&softirq_dev_lock, &rflags);
list_del_init(&entry->softirq_node);
- spinlock_irqrestore_release(&softirq_dev_lock);
+ spinlock_irqrestore_release(&softirq_dev_lock, rflags);
}

void ptdev_init(void)
diff --git a/hypervisor/debug/logmsg.c b/hypervisor/debug/logmsg.c
index eb43c5a..b39d0f1 100644
--- a/hypervisor/debug/logmsg.c
+++ b/hypervisor/debug/logmsg.c
@@ -47,13 +47,13 @@ static int do_copy_earlylog(struct shared_buf *dst_sbuf,
{
uint32_t buf_size, valid_size;
uint32_t cur_tail;
- spinlock_rflags;
+ uint64_t rflags = 0;

if ((src_sbuf->ele_size != dst_sbuf->ele_size)
&& (src_sbuf->ele_num != dst_sbuf->ele_num)) {
- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
printf("Error to copy early hvlog: size mismatch\n");
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
return -EINVAL;
}

@@ -87,12 +87,11 @@ void init_logmsg(__unused uint32_t mem_size, uint32_t flags)
void do_logmsg(uint32_t severity, const char *fmt, ...)
{
va_list args;
- uint64_t timestamp;
+ uint64_t timestamp, rflags = 0;
uint16_t pcpu_id;
bool do_console_log;
bool do_mem_log;
char *buffer;
- spinlock_rflags;

do_console_log = (((logmsg.flags & LOG_FLAG_STDOUT) != 0U) &&
(severity <= console_loglevel));
@@ -129,12 +128,12 @@ void do_logmsg(uint32_t severity, const char *fmt, ...)

/* Check if flags specify to output to stdout */
if (do_console_log) {
- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);

/* Send buffer to stdout */
printf("%s\n\r", buffer);

- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
}

/* Check if flags specify to output to memory */
@@ -169,11 +168,11 @@ void do_logmsg(uint32_t severity, const char *fmt, ...)

void print_logmsg_buffer(uint16_t pcpu_id)
{
- spinlock_rflags;
char buffer[LOG_ENTRY_SIZE + 1];
int read_cnt;
struct shared_buf **sbuf;
int is_earlylog = 0;
+ uint64_t rflags = 0;

if (pcpu_id >= phys_cpu_num) {
return;
@@ -187,13 +186,13 @@ void print_logmsg_buffer(uint16_t pcpu_id)
&per_cpu(sbuf, pcpu_id)[ACRN_HVLOG];
}

- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
if ((*sbuf) != NULL) {
printf("CPU%hu: head: 0x%x, tail: 0x%x %s\n\r",
pcpu_id, (*sbuf)->head, (*sbuf)->tail,
(is_earlylog != 0) ? "[earlylog]" : "");
}
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);

do {
uint32_t idx;
@@ -212,8 +211,8 @@ void print_logmsg_buffer(uint16_t pcpu_id)
idx = (read_cnt < LOG_ENTRY_SIZE) ? read_cnt : LOG_ENTRY_SIZE;
buffer[idx] = '\0';

- spinlock_irqsave_obtain(&(logmsg.lock));
+ spinlock_irqsave_obtain(&(logmsg.lock), &rflags);
printf("%s\n\r", buffer);
- spinlock_irqrestore_release(&(logmsg.lock));
+ spinlock_irqrestore_release(&(logmsg.lock), rflags);
} while (read_cnt > 0);
}
diff --git a/hypervisor/include/arch/x86/cpu.h b/hypervisor/include/arch/x86/cpu.h
index db928ce..b047aa4 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -418,13 +418,6 @@ void stop_cpus(void);
*timestamp_ptr = ((uint64_t)tsh << 32) | tsl; \
}

-/* Define variable(s) required to save / restore architecture interrupt state.
- * These variable(s) are used in conjunction with the ESAL_AR_INT_ALL_DISABLE()
- * and ESAL_AR_INT_ALL_RESTORE() macros to hold any data that must be preserved
- * in order to allow these macros to function correctly.
- */
-#define CPU_INT_CONTROL_VARS uint64_t cpu_int_value
-
/* Macro to save rflags register */
#define CPU_RFLAGS_SAVE(rflags_ptr) \
{ \
@@ -450,9 +443,9 @@ void stop_cpus(void);
* defined below and CPU_INT_CONTROL_VARS defined above.
*/

-#define CPU_INT_ALL_DISABLE() \
+#define CPU_INT_ALL_DISABLE(p_rflags) \
{ \
- CPU_RFLAGS_SAVE(&cpu_int_value); \
+ CPU_RFLAGS_SAVE(p_rflags); \
CPU_IRQ_DISABLE(); \
}

@@ -463,9 +456,9 @@ void stop_cpus(void);
* NOTE: This macro is used in conjunction with CPU_INT_ALL_DISABLE
* and CPU_INT_CONTROL_VARS defined above.
*/
-#define CPU_INT_ALL_RESTORE() \
+#define CPU_INT_ALL_RESTORE(rflags) \
{ \
- CPU_RFLAGS_RESTORE(cpu_int_value); \
+ CPU_RFLAGS_RESTORE(rflags); \
}

/*
diff --git a/hypervisor/include/lib/spinlock.h b/hypervisor/include/lib/spinlock.h
index 613e4b0..d6383bc 100644
--- a/hypervisor/include/lib/spinlock.h
+++ b/hypervisor/include/lib/spinlock.h
@@ -63,18 +63,15 @@ static inline void spinlock_release(spinlock_t *lock)

#endif /* ASSEMBLER */

-#define spinlock_rflags unsigned long cpu_int_value
-
-#define spinlock_irqsave_obtain(l) \
+#define spinlock_irqsave_obtain(lock, p_rflags) \
do { \
- CPU_INT_ALL_DISABLE(); \
- spinlock_obtain(l); \
+ CPU_INT_ALL_DISABLE(p_rflags); \
+ spinlock_obtain(lock); \
} while (0)

-#define spinlock_irqrestore_release(l) \
+#define spinlock_irqrestore_release(lock, rflags) \
do { \
- spinlock_release(l); \
- CPU_INT_ALL_RESTORE(); \
+ spinlock_release(lock); \
+ CPU_INT_ALL_RESTORE(rflags); \
} while (0)
-
#endif /* SPINLOCK_H */
--
2.7.4


Re: [PATCH V2] hv: vuart: fix 'Shifting value too far'

Junjie Mao
 

Shiqing Gao <shiqing.gao@...> writes:

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@...>
Reviewed-by: Junjie Mao <junjie.mao@...>

For the rest violations related to integral types, please follow up with
another patch. Thanks.

--
Best Regards
Junjie Mao

---
hypervisor/debug/vuart.c | 42 ++++++++++++++++++++--------------------
hypervisor/include/debug/vuart.h | 20 +++++++++----------
2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/hypervisor/debug/vuart.c b/hypervisor/debug/vuart.c
index 57e2611..cac6fc0 100644
--- a/hypervisor/debug/vuart.c
+++ b/hypervisor/debug/vuart.c
@@ -133,12 +133,13 @@ static void vuart_toggle_intr(struct vuart *vu)
}
}

-static void vuart_write(__unused struct vm_io_handler *hdlr,
- struct vm *vm, uint16_t offset_arg,
- __unused size_t width, uint32_t value)
+static void vuart_write(__unused struct vm_io_handler *hdlr, struct vm *vm,
+ uint16_t offset_arg, __unused size_t width, uint32_t value)
{
uint16_t offset = offset_arg;
struct vuart *vu = vm_vuart(vm);
+ uint8_t value_u8 = (uint8_t)value;
+
offset -= vu->base;
vuart_lock(vu);
/*
@@ -146,19 +147,19 @@ static void vuart_write(__unused struct vm_io_handler *hdlr,
*/
if ((vu->lcr & LCR_DLAB) != 0) {
if (offset == UART16550_DLL) {
- vu->dll = value;
+ vu->dll = value_u8;
goto done;
}

if (offset == UART16550_DLM) {
- vu->dlh = value;
+ vu->dlh = value_u8;
goto done;
}
}

switch (offset) {
case UART16550_THR:
- fifo_putchar(&vu->txfifo, value);
+ fifo_putchar(&vu->txfifo, value_u8);
vu->thre_int_pending = true;
break;
case UART16550_IER:
@@ -166,26 +167,26 @@ static void vuart_write(__unused struct vm_io_handler *hdlr,
* Apply mask so that bits 4-7 are 0
* Also enables bits 0-3 only if they're 1
*/
- vu->ier = value & 0x0FU;
+ vu->ier = value_u8 & 0x0FU;
break;
case UART16550_FCR:
/*
* The FCR_ENABLE bit must be '1' for the programming
* of other FCR bits to be effective.
*/
- if ((value & FCR_FIFOE) == 0U) {
- vu->fcr = 0;
+ if ((value_u8 & FCR_FIFOE) == 0U) {
+ vu->fcr = 0U;
} else {
- if ((value & FCR_RFR) != 0U) {
+ if ((value_u8 & FCR_RFR) != 0U) {
fifo_reset(&vu->rxfifo);
}

- vu->fcr = value &
+ vu->fcr = value_u8 &
(FCR_FIFOE | FCR_DMA | FCR_RX_MASK);
}
break;
case UART16550_LCR:
- vu->lcr = value;
+ vu->lcr = value_u8;
break;
case UART16550_MCR:
/* ignore modem */
@@ -202,7 +203,7 @@ static void vuart_write(__unused struct vm_io_handler *hdlr,
*/
break;
case UART16550_SCR:
- vu->scr = value;
+ vu->scr = value_u8;
break;
default:
/*
@@ -218,14 +219,13 @@ done:
vuart_unlock(vu);
}

-static uint32_t vuart_read(__unused struct vm_io_handler *hdlr,
- struct vm *vm, uint16_t offset_arg,
- __unused size_t width)
+static uint32_t vuart_read(__unused struct vm_io_handler *hdlr, struct vm *vm,
+ uint16_t offset_arg, __unused size_t width)
{
uint16_t offset = offset_arg;
- char iir, reg;
- uint8_t intr_reason;
+ uint8_t iir, reg, intr_reason;
struct vuart *vu = vm_vuart(vm);
+
offset -= vu->base;
vuart_lock(vu);
/*
@@ -295,7 +295,7 @@ static uint32_t vuart_read(__unused struct vm_io_handler *hdlr,
done:
vuart_toggle_intr(vu);
vuart_unlock(vu);
- return reg;
+ return (uint32_t)reg;
}

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

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

vu->active = false;
vu->base = COM1_BASE;
diff --git a/hypervisor/include/debug/vuart.h b/hypervisor/include/debug/vuart.h
index 38499a1..ea0754c 100644
--- a/hypervisor/include/debug/vuart.h
+++ b/hypervisor/include/debug/vuart.h
@@ -39,16 +39,16 @@ struct fifo {
};

struct vuart {
- char data; /* Data register (R/W) */
- char ier; /* Interrupt enable register (R/W) */
- char lcr; /* Line control register (R/W) */
- char mcr; /* Modem control register (R/W) */
- char lsr; /* Line status register (R/W) */
- char msr; /* Modem status register (R/W) */
- char fcr; /* FIFO control register (W) */
- char scr; /* Scratch register (R/W) */
- char dll; /* Baudrate divisor latch LSB */
- char dlh; /* Baudrate divisor latch MSB */
+ uint8_t data; /* Data register (R/W) */
+ uint8_t ier; /* Interrupt enable register (R/W) */
+ uint8_t lcr; /* Line control register (R/W) */
+ uint8_t mcr; /* Modem control register (R/W) */
+ uint8_t lsr; /* Line status register (R/W) */
+ uint8_t msr; /* Modem status register (R/W) */
+ uint8_t fcr; /* FIFO control register (W) */
+ uint8_t scr; /* Scratch register (R/W) */
+ uint8_t dll; /* Baudrate divisor latch LSB */
+ uint8_t dlh; /* Baudrate divisor latch MSB */

struct fifo rxfifo;
struct fifo txfifo;


Re: [PATCH 0/3] hv: debug: add the hypervisor NPK log

Yan, Like
 

It looks quite similar with acrn log, I am think if we could removed one, or merge them.
shall we have an offline discussion first?

Thanks,
Like

On Thu, Aug 09, 2018 at 09:54:45AM +0800, Zhi Jin wrote:
This series of patches implement a log destination for the hypervisor,
npk_log, which is similar to the console_log and the mem_log.
The Intel Trace Hub (aka. North Peak, NPK) is a trace aggregator for
Software, Firmware, and Hardware. The npk_log utilizes the NPK by
writing the hypervisor logs directly to the MMIO address of the
reserved NPK master, so that the hypervisor logs can be consolidated
together with any other logs sent to the NPK from other parts of the
virtualization platform with an unified timestamp.
In order to enable/disable/configure the npk_log from the SOS kernel,
a hypercall HC_SETUP_HV_NPK_LOG (the first patch of the series) and a
kernel driver (covered by another patch series) are also added.

+-------------------------------------+
| Interfaces exposed by the driver |
SOS | U: and used by the applications |
| +----------------^----------------+ |
| | |
| K: +---------v---------+ |
| | hv_npk_log driver | |
| +---------^---------+ |
+------------------|------------------+
| HC_SETUP_HV_NPK_LOG
+------------------|------------------+
HV | +-------v-------+ |
| | npk_log | |
| +-------+-------+ |
+------------------|------------------+
|
+-------------+----v----+-------------+
NPK MMIO | | /////// | |
| | /////// | |
+-------------+----+----+-------------+
|
+---+ The Master/Channel reserved
for the hypervisor NPK logs

Zhi Jin (3):
hv: add a hypercall for the hypervisor NPK log
hv: add mmio functions for 64bit values
hv: debug: add the hypervisor NPK log

hypervisor/arch/x86/Kconfig | 6 +-
hypervisor/arch/x86/guest/vmcall.c | 4 ++
hypervisor/common/hypercall.c | 22 +++++++
hypervisor/debug/logmsg.c | 9 ++-
hypervisor/debug/npk_log.c | 110 +++++++++++++++++++++++++++++++
hypervisor/debug/shell.c | 25 ++++---
hypervisor/debug/shell_internal.h | 3 +-
hypervisor/include/arch/x86/io.h | 36 ++++++++++
hypervisor/include/arch/x86/per_cpu.h | 1 +
hypervisor/include/common/hypercall.h | 11 ++++
hypervisor/include/debug/logmsg.h | 2 +
hypervisor/include/debug/npk_log.h | 58 ++++++++++++++++
hypervisor/include/hv_debug.h | 1 +
hypervisor/include/public/acrn_hv_defs.h | 23 +++++++
14 files changed, 298 insertions(+), 13 deletions(-)
create mode 100644 hypervisor/debug/npk_log.c
create mode 100644 hypervisor/include/debug/npk_log.h

--
1.9.1